Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

argparse with --ignore-unknown fails on unknown grouped short options #8637

Closed
guillonb opened this issue Jan 14, 2022 · 3 comments
Closed
Labels
bug Something that's not working as intended
Milestone

Comments

@guillonb
Copy link

guillonb commented Jan 14, 2022

Here is my configuration:

$ sh -c 'env HOME=$(mktemp -d) fish'
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish
$ fish --version
fish, version 3.3.1
$ echo $version
3.3.1
$ uname -a
Linux Gacrux 5.10.0-10-amd64 #1 SMP Debian 5.10.84-1 (2021-12-08) x86_64 GNU/Linux
$ echo $TERM
xterm-256color

Here is a minimal working example:

$ argparse -i 'h' -- '-o a' -h
$ set -l

Obtained output:

argv '-o a'  '-h'

Expected output:

_flag_h -h
argv '-o a'

(Note that the output is as expected when '-h' is given before '-o a'.)

Here is a use case. I have a function, that call a command, say cmd, which accepts options. My function allows a user to pass some options to cmd, by using the -c option (of my function) which takes one argument, namely the string of options to pass to cmd. Before parsing the option of my function, I want to consider printing help, so that it will be successfully printed even if a wrong option is in used (I like to do my function this way). My code (assuming the command cmd):

$ function myfunc
      argparse -i 'h/help' -- $argv
      set -q _flag_h
      and echo "Usage: myfunc [-c OPT4CMD] [ARG...]"
      and return
      
      argparse 'c/cmd-opt=' -- $argv
      or return
      
      cmd (string split -- ' ' $_flag_c) $argv
  end

(Of course in this simple example, there is no real need to write the function.)


My test showed me that actually an argument starting with '-' but containing a space is still considered as an option. This is the case for several fish buitlins, such as string for instance, but not for others, such as echo.

$ string split ' ' '-s a'

returns an error (code 2) with message (stderr):

string split: Unknown option “-s a”

(Type 'help string' for related documentation)

while

$ echo '-s a'

just successfully outputs

-s a

Would it not be more reasonable to never consider an argument starting with - as but containing a space (or other special characters) as an option?

@faho faho added the bug Something that's not working as intended label Jan 14, 2022
@faho
Copy link
Member

faho commented Jan 14, 2022

Ah, okay. What happens here is that the option grouping logic kicks in and -s a is recognized as three separate, unknown, short options: -s, - and -a.

Then argparse/our getopt implementation keeps incrementing the option index for these and tries to dismiss that many options. This means that, e.g. argparse -i 'h' -- '-o a' -h -h -h starts counting the third -h as an option.

Now the getopt API is awful so I don't actually know how we would express that the unknown option is the o part and then the part and then the a part. I'd have to check. Presumably we'd have to redo some of the permutation getopt does for known options.

Would it not be more reasonable to never consider an argument starting with - as but containing a space (or other special characters) as an option?

No, because options can have attached arguments.

E.g. commandline -C" something" will have the -C option with the argument something (the quotes don't reach the command!).

It's also technically possible to have an option called - . So we can't be sure if you failed to give an option or if you failed to explain that this option can have an argument.

And with --ignore-unknown, if you know the option -a and you get the string -ao" foo" (with the quotes then stripped), did -a need an argument or does -o read one or does - exist?

@faho
Copy link
Member

faho commented Jan 14, 2022

To wit: The space is unnecessary.

> argparse -i h -- -foo -h
> echo $argv
-foo -h
> echo $_flag_h
# nothing

It seems we forget to tell getopt to go to the next argument in argv once it found an unknown option. That should be simple to fix.

@faho faho closed this as completed in 0781473 Jan 15, 2022
@faho faho added this to the fish 3.4.0 milestone Jan 15, 2022
@faho faho changed the title argparse with --ignore-unknown bug when one argument starts with - and contains a space argparse with --ignore-unknown fails on unknown grouped short options Jan 15, 2022
@guillonb
Copy link
Author

Ok. I'm happy this was simple to fix. Thank you for the answer.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended
Projects
None yet
Development

No branches or pull requests

2 participants