Re: Disallowing args
On Jun 18, 2010, at 12:26 PM, Wayne Davison wrote: > On Thu, Jun 17, 2010 at 9:21 AM, Jeff Johnson wrote: > Meanwhile in the future, if you could send a patch against some POPT release > that I can find > > Sorry about that. I sent you a patch for the rsync popt by mistake. Here's > one I just made against 1.16 that now includes a couple tests for the arg > rejection. > > http://opencoder.net/arg-error.patch > Oooh, test cases too. Thank you! (aside) I'm gonna need some help from non-English speakers in order to get test cases for --help column alignment (and generally handling wide characters correctly) This dumb 'merikkan cannot be expected to type up reproducers for problems that he cannot see or understand. Sure I can see that the --help columns don't line up even with ired old bifocal encrusted eyes. That ain't what I'm asking for ... 73 de Jeff
Re: Disallowing args
On Thu, Jun 17, 2010 at 9:21 AM, Jeff Johnson wrote: > Meanwhile in the future, if you could send a patch against some POPT > release that I can find > Sorry about that. I sent you a patch for the rsync popt by mistake. Here's one I just made against 1.16 that now includes a couple tests for the arg rejection. http://opencoder.net/arg-error.patch ..wayne..
Re: Disallowing args
On Jun 5, 2010, at 11:39 AM, Wayne Davison wrote: > Here's something that was recently fixed for the popt that is included > with rsync: rejecting an arg to an option that doesn't take an arg. > > Attached is a patch. A new error code, POPT_ERROR_UNWANTEDARG, was > created to make the error message nice. This handles both -l=value > and --long-arg=value where neither one is supposed to take a value. > > ..wayne.. > Hmmm, I'll get this "fix" worked in down the road. Certainly a bug. (aside) All this code is actively being simplified for POPT 2.0 so that I can more easily follow the logic path. Meanwhile in the future, if you could send a patch against some POPT release that I can find (I tried all the way back to the popt-1_11-release tag, patch doesn't apply), or at least provide some hint about what code repository the patch applies to (the patch itself seems to have been edited), that would certainly help. Please also note that I won't accept any patch that doesn't pass "make check" regression tests. If you _REALLY_ want to make sure that odd POPT corner cases like args given to an option return an error, adding a reproducer to the test cases too. 73 de Jeff __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Disallowing args
On Jun 5, 2010, at 12:42 PM, Danny Sung wrote: > On 06/05/2010 8:56 AM, Jeff Johnson wrote: > >> (aside) >> Anything you want to see in POPT 2.0? I'm collecting features ... >> > > Since you're collecting features... =) > > One thing I'd like is to extend the help/usage capability just a little. > Well reworking --help is one of the primary motivations for POPT 2.0. Hysterically, encoding was rammed into POPT by GNOME way back when. While the scheme is workable, the original change _HAD_ to be done as an addition to preserve ABI, and so all of the i18n for --help is rather awkwardly done. To make matters worse, another patch from GNOME wanted POPT to undertake localization transforms from UTF8 to whatever was in LC_ALL etc. While iconv(3) makes the character encoding transform as "trivial" as deciding the to <-> from locales, its hardly trivial to make that decision reliably based solely on a desired LC_ALL environment variable (the from locale cannot be determined reliably, and hackery like trying various from locales like glib does is hardly sound engineering). The killer is that the data needed to find --help alignment reproducers is application resident, and application chosen, and the GNOME developers who forced the iconv(3) into POPT just aren't helping with reproducers. I have no interest in supporting POPT functionality that noone is willing to help maintain any more. So the short answer is: All of the --help handling needs to be scrapped and reworked in POPT 2.0. For starters, 30-40% of the code is --help related, and _ALL_ of the bug reports I've heard for years are But --help columns don't align! Enough already ... bring on the --help bulldozers! > So I'd like to be able to have more descriptive usage parameters (eg. to > cover left-over arguments). I addition it'd be nice to have a little > description/summary of what the program to do. I realize you can do this > with a custom help function. But it'd be nice if these were standard > elements. > > Other niceties might be: > - a way to indicate parameters enabled by default (eg. having a '*' next to > them in the help) There is already #define POPT_ARGFLAG_SHOW_DEFAULT 0x0080U /*!< show default value in --help */ as well as #define POPT_ARGFLAG_OPTIONAL 0x1000U /*!< arg may be missing */ wired into --help output. > > - An additional structure that could provide detailed help on argDescription > elements. For example, rpm has an option: > --queryformat=QUERYFORMATuse the following query format > It'd be nice if there were a section of help that could describe what > QUERYFORMAT could be. So obviously it's not going to be a full man page, but > perhaps it could just show supported % format options or something like that. This is a different issue. There is already the 6th/7th fields in POPT tables, the problem is really "information overload" from --help. At some point man(1) or info(1) is a better approach than --help, particularly for RPM which has an entire eco-system of option processing and far too many options to be reasonably displayed with --help (because the info is not very helpful). (aside) Note that RPM is _ROUTINELY adding #define POPT_ARGFLAG_DOC_HIDDEN 0x4000U /*!< don't show in help/usage */ there's zillions of options in RPM that noone knows about. My guess is that there's more "hidden" than displayed options these days, usually disablers that noone ever really needs to use. > I use something like this in my code, but I have specific keys like > "[replaceme]" that I convert. And I put just the acceptable keys in the help > cause I just need a quick reminder of what they are. But it clutters the > option help a little. I'd be fine with specifying "FORMATSTRING" in the > option help. Then have perhaps an arg help down below that shows what values > FORMATSTRING understands. > > > I'm not sure exactly how you could support these without breaking > compatability with existing apps. Perhaps a new datatype something like: > FYI: POPT 2.0 is all about breaking compatibility. There's only so much that can be done with obscure overloadings of the existing 7-tuple in popt table entries. > enum poptOptionType { > POPT_OPTION, > POPT_ARG, > POPT_USAGE, > }; > > union poptDetailedOption { > poptOptionType optType; > struct poptShortOption; > struct poptArgHelp; > struct poptUsageHelp; > } > > struct poptShortOption { /* same as poptOption but with a type field */ >poptOptionType optType; >const char * longName; >char shortName; >int argInfo; >void * arg; >int val; >const char * descrip; >const char * argDescrip; > }; > > > I'm not sure this would give the desired effect.. but the thought would be > that it'd turn your options table to something like this: > > poptDetailedOption optionsTable[] = { >{ POPT_OPTION, "bps", 'b', POPT_ARG_INT, &speed, 0, >
Re: Disallowing args
On Jun 5, 2010, at 12:48 PM, Wayne Davison wrote: > On Sat, Jun 5, 2010 at 8:56 AM, Jeff Johnson wrote: > but >--foo bar > returns bar in the argument list? > > Yes. The user may well have wanted it to be in the arg list. There's no way > for the program to know that the user didn't just toss some options in the > middle of some args (which I do all the time these days, like starting my > rsync source/dest args, and then tossing in a --remove-source-files, > --backup, or what-not at wherever I am in the list), so I wouldn't want to > see an error for something like this: > > rsync -aiv --del file --remove-source-args some/dir host:/dest/dir > > ... just because --del doesn't take an arg. > > The other way to "fix" the error is to morph "--foo=bar" behavior to be > identical to "--foo bar", i.e. an extra argument failure. > > I don't see how that would work for something like rsync that takes any > number of command-line args outside the options. > Well determistic behavior "works", just not very useful ;-) I'm not arguing, just there's issues like POSIX_ME_HARDER that get factored into "--foo=bar" error handling, all mind numbingly tedious. Your patch is likely what I will add, I'm a very lazy schmuck. > Anything you want to see in POPT 2.0? I'm collecting features ... > > A couple ideas off the top of my head: > An incrementing option -- repeated use adds 1 to the variable instead of > setting it to the same value. How about a full blown RPN calculator to handle not just increment/decrement, but all arithmetic operations, on option values. infix <-> postfix just isn't that hard, and the whole mess is just a teensy stack and a switch. I've got several RPN calculators floating around in RPM code these days that could be dropped into POPT without any effort whatsoever. Nor is a RPM calculator very hard to implement. > Multiple long names separated by "|" in the long-name string (though that > could really just be defined as an alias, it might be nice to auto-gen the > alias). There's already Bloom filters fin POPT 1.16 for opaquely handling multiple option value strings (the "bar" in "--foo bar"). That partly addresses the need for fewer popt table entries/aliases/execs. (aside) The usage case is for RPM which has >100 hash algorithm names buried into popt tables, and I need to collapse down to a single popt table entry for my own maintenance sanity. Full pattern matching, either *RE or fnmatch(3), would not be hard. The pdksh extensions to fnmatch globs would handle your specific alternation request. And POPT is already linking fnmatch(3). If you have any other RFE's just drop a note here ... and blame Rusty Russell for encouraging POPT 2.0 ;-) 73 de Jeff
Re: Disallowing args
On 06/05/2010 8:56 AM, Jeff Johnson wrote: (aside) Anything you want to see in POPT 2.0? I'm collecting features ... Since you're collecting features... =) One thing I'd like is to extend the help/usage capability just a little. So I'd like to be able to have more descriptive usage parameters (eg. to cover left-over arguments). I addition it'd be nice to have a little description/summary of what the program to do. I realize you can do this with a custom help function. But it'd be nice if these were standard elements. Other niceties might be: - a way to indicate parameters enabled by default (eg. having a '*' next to them in the help) - An additional structure that could provide detailed help on argDescription elements. For example, rpm has an option: --queryformat=QUERYFORMATuse the following query format It'd be nice if there were a section of help that could describe what QUERYFORMAT could be. So obviously it's not going to be a full man page, but perhaps it could just show supported % format options or something like that. I use something like this in my code, but I have specific keys like "[replaceme]" that I convert. And I put just the acceptable keys in the help cause I just need a quick reminder of what they are. But it clutters the option help a little. I'd be fine with specifying "FORMATSTRING" in the option help. Then have perhaps an arg help down below that shows what values FORMATSTRING understands. I'm not sure exactly how you could support these without breaking compatability with existing apps. Perhaps a new datatype something like: enum poptOptionType { POPT_OPTION, POPT_ARG, POPT_USAGE, }; union poptDetailedOption { poptOptionType optType; struct poptShortOption; struct poptArgHelp; struct poptUsageHelp; } struct poptShortOption { /* same as poptOption but with a type field */ poptOptionType optType; const char * longName; char shortName; int argInfo; void * arg; int val; const char * descrip; const char * argDescrip; }; I'm not sure this would give the desired effect.. but the thought would be that it'd turn your options table to something like this: poptDetailedOption optionsTable[] = { { POPT_OPTION, "bps", 'b', POPT_ARG_INT, &speed, 0, "signaling rate in bits-per-second", "BPS" }, { POPT_ARG, "FORMATSTRING", "Possible formatting options are [foo] [bar]" }, { POPT_USAGE, "infile [outfile]" }, { POPT_SUMMARY, "This program converts your data to something" }, I don't think the union will allow it to look exactly like that. In fact it may not even be possible to initialize it that way(?) Maybe we'd have to do something wonky with macros to make it look nice. But you get the idea. If you wanted to get really fancy the "POPT_USAGE" example I gave could be separated into something like this: { POPT_EXTRAARG, "infile", 1 /* required */, "specify input filename", "stdin" /* default value */ }, { POPT_EXTRAARG, "outfile", 0 /* optional */, "file to output" /* description */ "stdout" /* default value */ } This would allow the autohelp to generate "standard" extra arg displays (eg. [] for optional parameters, etc.). Admittedly this feature may be a bit questionable as many programs may have rather complex "extra args". But I think a lot of programs fall into this simple category as well. Anyway, just a few thoughts... __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Disallowing args
On Sat, Jun 5, 2010 at 8:56 AM, Jeff Johnson wrote: > but >--foo bar > returns bar in the argument list? > Yes. The user may well have wanted it to be in the arg list. There's no way for the program to know that the user didn't just toss some options in the middle of some args (which I do all the time these days, like starting my rsync source/dest args, and then tossing in a --remove-source-files, --backup, or what-not at wherever I am in the list), so I wouldn't want to see an error for something like this: rsync -aiv --del file --remove-source-args some/dir host:/dest/dir ... just because --del doesn't take an arg. The other way to "fix" the error is to morph "--foo=bar" behavior to be > identical to "--foo bar", i.e. an extra argument failure. > I don't see how that would work for something like rsync that takes any number of command-line args outside the options. Anything you want to see in POPT 2.0? I'm collecting features ... A couple ideas off the top of my head: - An incrementing option -- repeated use adds 1 to the variable instead of setting it to the same value. - Multiple long names separated by "|" in the long-name string (though that could really just be defined as an alias, it might be nice to auto-gen the alias). ..wayne..
Re: Disallowing args
On Jun 5, 2010, at 11:39 AM, Wayne Davison wrote: > Here's something that was recently fixed for the popt that is included > with rsync: rejecting an arg to an option that doesn't take an arg. > > Attached is a patch. A new error code, POPT_ERROR_UNWANTEDARG, was > created to make the error message nice. This handles both -l=value > and --long-arg=value where neither one is supposed to take a value. > Hmm, so --foo=bar returns POPT_ERROR_UNWANTEDARG but --foo bar returns bar in the argument list? The other way to "fix" the error is to morph "--foo=bar" behavior to be identical to "--foo bar", i.e. an extra argument failure. Lemme muddle a bit about whether the two forms should be treated differently or identically, that's purely a design issue. Both cases are an error no matter what. There's nothing wrong with your patch whatsoever, just that it introduces instant legacy incompatibility. *shrug* Thanks for the patch. (aside) Anything you want to see in POPT 2.0? I'm collecting features ... 73 de Jeff __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org