Re: getopt error message with a bare "-"
Den lör 2 dec. 2023 kl 11:08 skrev Graham Leggett : > On 28 Nov 2023, at 07:46, Daniel Sahlberg > wrote: > > Subversion is using APR's apr_getopt_long for command line processing. > > A user tried to supply "-" for the filename with the intention of using > stdin (Subversion has support for this in our code). However the user got > the following error message (with the first line coming from > apr_getopt_long and the second from Subversion's code): > > $ ./svnmucc put - file:///url/to/repository > svnmucc: invalid option: > Type 'svnmucc --help' for usage. > > We found the solution, to use "--" to disable option processing, so no > discussion around this is needed. > > In the thread, someone mentioned that it would be useful if the error > message could be improved so I looked into the APR code and have a > suggestion. > > The following code is responsible for the error message. In other places, > the error message contains the offending (long) option (third argument). > However since *p is always \0, checked just above, the error message will > just say "invalid option". I'm suggesting to change this to the fixed > string "-" instead. > [[[ > --- getopt.orig 2023-11-28 08:22:16.690508444 +0100 > +++ getopt.c2023-11-28 08:22:50.270297468 +0100 > @@ -272,7 +272,7 @@ > } > else > if (*p == '\0')/* Bare "-" is illegal > */ > -return serr(os, "invalid option", p, APR_BADCH); > +return serr(os, "invalid option", "-", APR_BADCH); > } > } > ]]] > > With this change, the error message will instead be: > $ ./svnmucc put - file:///url/to/repository > svnmucc: invalid option: - > Type 'svnmucc --help' for usage. > > with the "-" indicating which part of the command line is invalid. > > Original e-mail from our users@ list: > https://lists.apache.org/thread/mbsx7rxpx07sh6sfs6xhbnvgo2wbfl0k > > Follow-up e-mail in our dev@ list: > https://lists.apache.org/thread/l7x22yp6kb71qlv3rn8tfmcrc5hk73r4 > > > Waves hi :) > Oh, hi! :) (Graham is the user I mentioned above, didn't know you were here ... ) > > +1 on the above message fix at the very least. I’ll commit it if there are > no objections. > > Another solution would be to add a switch to getopt.c that allows for - to > be counted not as a "start of short option string" but as a separate > argument, possibly hidden behind a switch in the apr_getopt_t struct. For > our use, that would be even better, but I assume it would require more code > (also on our side to remain backwards compatible with older APR versions). > > > Looks like apr_getopt_t is the right place for a flag. It seems a lot more > intuitive to have “-“ recognised as an argument rather than an invalid > option. > That's exactly how I saw it as well. Not sure if we (Subversion) would even make use of that option for a long time, since we need to be reasonably sure an uptodate version of APR is available before we release. > > Regards, > Graham > — > Kind regards Daniel >
Re: getopt error message with a bare "-"
On 28 Nov 2023, at 07:46, Daniel Sahlberg wrote: > Subversion is using APR's apr_getopt_long for command line processing. > > A user tried to supply "-" for the filename with the intention of using stdin > (Subversion has support for this in our code). However the user got the > following error message (with the first line coming from apr_getopt_long and > the second from Subversion's code): > > $ ./svnmucc put - file:///url/to/repository > svnmucc: invalid option: > Type 'svnmucc --help' for usage. > > We found the solution, to use "--" to disable option processing, so no > discussion around this is needed. > > In the thread, someone mentioned that it would be useful if the error message > could be improved so I looked into the APR code and have a suggestion. > > The following code is responsible for the error message. In other places, the > error message contains the offending (long) option (third argument). However > since *p is always \0, checked just above, the error message will just say > "invalid option". I'm suggesting to change this to the fixed string "-" > instead. > [[[ > --- getopt.orig 2023-11-28 08:22:16.690508444 +0100 > +++ getopt.c2023-11-28 08:22:50.270297468 +0100 > @@ -272,7 +272,7 @@ > } > else > if (*p == '\0')/* Bare "-" is illegal */ > -return serr(os, "invalid option", p, APR_BADCH); > +return serr(os, "invalid option", "-", APR_BADCH); > } > } > ]]] > > With this change, the error message will instead be: > $ ./svnmucc put - file:///url/to/repository > svnmucc: invalid option: - > Type 'svnmucc --help' for usage. > > with the "-" indicating which part of the command line is invalid. > > Original e-mail from our users@ list: > https://lists.apache.org/thread/mbsx7rxpx07sh6sfs6xhbnvgo2wbfl0k > > Follow-up e-mail in our dev@ list: > https://lists.apache.org/thread/l7x22yp6kb71qlv3rn8tfmcrc5hk73r4 Waves hi :) +1 on the above message fix at the very least. I’ll commit it if there are no objections. > Another solution would be to add a switch to getopt.c that allows for - to be > counted not as a "start of short option string" but as a separate argument, > possibly hidden behind a switch in the apr_getopt_t struct. For our use, that > would be even better, but I assume it would require more code (also on our > side to remain backwards compatible with older APR versions). Looks like apr_getopt_t is the right place for a flag. It seems a lot more intuitive to have “-“ recognised as an argument rather than an invalid option. Regards, Graham —
getopt error message with a bare "-"
Hi, Subversion is using APR's apr_getopt_long for command line processing. A user tried to supply "-" for the filename with the intention of using stdin (Subversion has support for this in our code). However the user got the following error message (with the first line coming from apr_getopt_long and the second from Subversion's code): $ ./svnmucc put - file:///url/to/repository svnmucc: invalid option: Type 'svnmucc --help' for usage. We found the solution, to use "--" to disable option processing, so no discussion around this is needed. In the thread, someone mentioned that it would be useful if the error message could be improved so I looked into the APR code and have a suggestion. The following code is responsible for the error message. In other places, the error message contains the offending (long) option (third argument). However since *p is always \0, checked just above, the error message will just say "invalid option". I'm suggesting to change this to the fixed string "-" instead. [[[ --- getopt.orig 2023-11-28 08:22:16.690508444 +0100 +++ getopt.c2023-11-28 08:22:50.270297468 +0100 @@ -272,7 +272,7 @@ } else if (*p == '\0')/* Bare "-" is illegal */ -return serr(os, "invalid option", p, APR_BADCH); +return serr(os, "invalid option", "-", APR_BADCH); } } ]]] With this change, the error message will instead be: $ ./svnmucc put - file:///url/to/repository svnmucc: invalid option: - Type 'svnmucc --help' for usage. with the "-" indicating which part of the command line is invalid. Original e-mail from our users@ list: https://lists.apache.org/thread/mbsx7rxpx07sh6sfs6xhbnvgo2wbfl0k Follow-up e-mail in our dev@ list: https://lists.apache.org/thread/l7x22yp6kb71qlv3rn8tfmcrc5hk73r4 Another solution would be to add a switch to getopt.c that allows for - to be counted not as a "start of short option string" but as a separate argument, possibly hidden behind a switch in the apr_getopt_t struct. For our use, that would be even better, but I assume it would require more code (also on our side to remain backwards compatible with older APR versions). Kind regards, Daniel Sahlberg
