oh-my-fish / plugin-argu

Sane and easy to use argument parser for Oh My Fish!
MIT License
21 stars 2 forks source link

Use printf instead of echo to parse short option #2

Open ryotako opened 7 years ago

ryotako commented 7 years ago

Hi, thank you for the good plugin! This is the nicest option parser for fish-shell.

I fixed short option parsing. In the current version, options -e, -n, -s, -E are not parsed because they are options of echo command.

argu e -- -e # output is ''
pjeby commented 7 years ago

I think maybe you mean printf "%s\n" $argv[1], as echo -E intentionally suppresses backslash escapes, but printf doesn't if you use user input as the format string.

ryotako commented 7 years ago

Thanks! I assumed that options contained only alphabets, numbers and -. As you pointed out, backslash escapes cannot be managed correctly in my change.

I just want to use short options -e, -n, -s and -E. How about this?

 # Match a short or long single option.
      case -e -n -s -E
          printf "$argv[1]\n"    
      case '--*' '-?'
          echo -E "$argv[1]"
      ...
pjeby commented 7 years ago

Why not just printf "%s\n" $argv[1]? It works for any value, always. Really, you should never pass a string beginning with a variable as the first non-option argument to either echo or printf. (And there's no reason to ever put variables in the first argument to printf unless you are doing something way more complicated than this library does.)

As for assuming that options only contain the right characters, bear in mind that user input can contain typos or malicious values intended to crash the program or make it misbehave.

So, most echo -Es in the function should be replaced with printf "%s\n" whatever, The only time it's safe to use echo is when the first non-option argument begins with a constant value that doesn't start with '-'. So for example, this is safe:

echo "Unknown option `$argv[1]'." >&2

because it begins with a constant that does not start with -. But anything that doesn't start that way should be using printf without any variables in the first argument. That is, never use printf "$something" because what you should be doing is printf "%s" $something. The constant parts go in the first argument, variable parts in the other arguments.

ryotako commented 7 years ago

Sorry for my late reply and poor English skill...

I did not understand well the behavior of printf. Now I think your suggestion is a perfect solution.

I had fixed it!

pjeby commented 7 years ago

No problem. Glad I could help.

zephraph commented 4 years ago

@sagebind would you mind reviewing? This one bit me too. :pray: