iffy / nim-argparse

Argument parsing for Nim
MIT License
122 stars 8 forks source link

Exception on `-h` | `--help` #10

Closed SolitudeSF closed 5 years ago

SolitudeSF commented 5 years ago
import argparse
let
  p = newParser("test"): discard
  opts = p.parse

with this minimal program will output the help contents and throw Error: unhandled exception: [ShortCircuit] when program invoked with any help flag. Am i missing something?

iffy commented 5 years ago

I had to think about this for a minute, but this is the expected behavior. This is the expected behavior because when -h or --help is given, the parser assumes that rather than parsing and continuing with the program, the help text should be displayed and the program (or section of the program) should abort.

So I would do:

import argparse
let
  p = newParser("test"): discard
try:
  let opts = p.parse()
except ShortCircuit:
  discard

However, there are some problems with this:

  1. It's not documented, so I should document it :)
  2. The name ShortCircuit isn't great. Something with Help in the name would be better.
  3. The help documentation is always printed out -- but you may want to do something else.
  4. Because of scoping and not knowing the type of opts, you can only use opts inside the try: statement, which might be annoying.

I'll see what I can do about these. Thanks for the report!

SolitudeSF commented 5 years ago

maybe the parser should also store if help is invoked in some special field, so user can manually check it and decide what to do?

iffy commented 5 years ago

I'm just re-reading https://nim-lang.org/docs/tut2.html#exceptions and thinking maybe using an exception here is not the Nim way to do things:

A convention is that exceptions should be raised in exceptional cases: For example, if a file cannot be opened, this should not raise an exception since this is quite common (the file may not exist).

Perhaps the result of parse should contain what is now returned as opts. So like this:

import argparse
let
  p = newParser("test"):
    flag("-a")
  res = p.parse(@["-a"])
assert res.help_flag == false
assert res.opts.a == true

The API for run() would not change.

What do you think of that?

SolitudeSF commented 5 years ago

opts already contains some special field help__special, would there be room for one more? introducing wrapper just for one value seems like overkill. or parse could return tuple of current opts and help flag.

iffy commented 5 years ago

What if I renamed help__special to help__shortcircuit and then documented that. Would that work?

SolitudeSF commented 5 years ago

is help__special that help invocation flag? documenting it would work, just exception has to go. also whats the reason for double underscore? it just triggers style check.

iffy commented 5 years ago

Hmmm... and what should it return in the case of subcommands?

import argparse
let
  p = newParser("test"):
    command("something"):
      discard
  res = p.parse(@["something", "-h"])

The reason for the double underscore was to avoid conflicts with user-defined keys. And since users can't use double underscores, it works well :)

I think I'll just name the flag .help and remove the exception.

SolitudeSF commented 5 years ago

underscores are ignored completely.

import argparse
let
  p = newParser("test"):
    flag("--helpspecial")
  opts = p.parse

this triggers Error: attempt to redefine: 'help__special' but thats offtopic.

I think I'll just name the flag .help and remove the exception.

that would be the most expected behavior. i didn't dig into subcommands, so i dont know how should they behave.

SolitudeSF commented 5 years ago

great, thank you. bumping a minor version would be nice.

iffy commented 5 years ago

Done! Thanks for reminding me :)

SolitudeSF commented 5 years ago

thanks for the great library. actually gonna switch now from parseopt to this in my projects.

skellock commented 5 years ago

Same here. I've got a crappy diy solution I've never been happy with. This is much better than what I've done.