bug#29617: `seq 1 --help' doesn't give help

[chadweisman]

On 2/19/2018 at 8:35 PM, "Pádraig Brady" <address@hidden> wrote:
>
>On 18/02/18 10:41, address@hidden wrote:
>> 
>>> On 12/08/2017 11:38 AM, Eric B wrote:
>>>> Hello,
>>>>
>>>> I am using coreutils version 8.27 on Fedora, and I don't see 
>this fixed in 
>>>> 8.28's NEWS.
>>>>
>>>> $ seq 1 --help
>>>> seq: invalid floating point argument: ‘--help’
>>>> Try 'seq --help' for more information.
>> 
>>> Interesting bug!
>> 
>>>>
>>>> We should be able to put the options anywhere and not 
>necessarily before any 
>>>> arguments.
>> 
>>> Yes, when possible.
>> 
>>>> And even if not (e.g. POSIX conformance overrides,)
>> 
>>> POSIX does say you have to write 'foo -- --help' if you want to
>>> guarantee that --help is treated as a literal argument rather 
>than
>>> option, but it also says that the moment you specify '--help' 
>(or any
>>> other parameter starting with two dashes without the -- end-of-
>options
>>> parameter), you are already in undefined territory.  So we can 
>do
>>> whatever we want when encountering '--help' - which is part of 
>the
>>> reason WHY the GNU project prefers making 'foo args --help' 
>print help
>>> output where possible.
>> 
>>>> --help should be handled specially to conform to the GNU 
>coding standards. [1]
>> 
>>> Yes.
>> 
>>> But the reason that it fails is because we use getopt_long(...,
>>> "+f:s:w") - where the leading '+' specifically requests that we 
>NOT
>>> allow option reordering.  Why? Because 'seq' is MOST useful if 
>it can
>>> parse negative numbers easily.  We deemed it more important to 
>support
>>> 'seq 2 -1 1' without requiring the user to write 'seq -- 2 -1 
>1' - but
>>> in doing so, it also means that we can't reorder options, so 
>any obvious
>>> non-option (like '1' in your example) makes all other parameters
>>> non-options (including '--help' in your example).
>> 
>>> It might be possible to do a two-pass parse over argv: one that 
>looks
>>> just for --help (and where treating -1 as an option is a no-
>op), and the
>>> second that actually parses things in order now that it knows --
>help is
>>> not present.  But that's a lot of code to add for a corner 
>case, so I
>>> won't be writing the patch; but I also won't turn it down if 
>someone
>>> else wants to write the patch.
>> 
>> Hello,
>> 
>> I've taken a stab at fixing this problem because it affects me 
>fairly often.
>> 
>> Instead of using a two-pass system, I check if any of the args 
>we scan are --help or --version and bail if we see either.  See 
>attached patch.
>> 
>> The bad side of this approach is that the -f, -s, and -w options 
>and their associated long options aren't handled so `seq 1 -w 2 
>10` still shows an error.  Also, it's a kludgy sort of fix, so I 
>completely understand why you wouldn't want to include it.  But at 
>least it's a step in the right direction to give help when we can 
>instead of an error.
>> 
>> Chad
>> 
>
>Thanks for looking at this again.
>You attached the wrong patch.
>

Wow, did I ever!  Sorry for that.

>A helper function to prescan argv to look for --help and call 
>usage()
>or --version and call version_etc(), while bailing itself
>if it encounters '--', does seem useful to have in seq.
>Though note it probably shouldn't support abbreviations like --h 
>etc.
>
>BTW other commands that use '+' with getopt() to exist option
>processing early are tr, basename, pathchk, printf.
>Though all those could interpret --option as a valid argument
>and so wouldn't be appropriate to treat like this.
>Likewise for all the commands the exec, like chroot, env, ...
>as --options are very often passed to the delegate commands.
>

I also tried a two-pass approach but it ended up being pretty messy.  I can't 
help but think using argp would make this particular problem easier to handle.

Chad

coreutils-seq.patch
Description: Binary data