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

Various arg_botcmd improvements #516

Merged
merged 4 commits into from
Oct 26, 2015
Merged

Conversation

zoni
Copy link
Member

@zoni zoni commented Oct 26, 2015

This PR introduces a number of enhancements to how the arg_botcmd decorator works:

  • Ensure arg_botcmd error and help output goes into chat and avoid raising SystemExit

By default, argparse will print errors, usage and help text directly to
stdout/stderr. It will also raise SystemExit to make the program stop
with an appropriate error code.

While this is appropriate for a CLI program, it is horrible for Err. The
SystemExit causes some strange behaviour where it doesn't make the bot
exit, but does prevent some processing (!restart no longer works, for
example) and the printing to stdout/stderr means you get no feedback
when issuing commands with incorrect arguments.

With these changes, all this output is nicely returned to the user,
instead.

  • Ensure the command name is used in the usage as well, instead of the
    default sys.arv[0] which resulted in one of err.py, errbot or
    run_tests.py.

  • Convert — to -- when parsing arg_botcmds

    Some clients (looking at you, Telegram) convert certain ASCII character
    combinations to nicer-looking unicode representations, which break the
    argument parser. Account for this and undo the conversion to avoid user
    frustration.

  • Make "argument unpacking" on arg_botcmd optional

With this change it becomes possible to define a single args argument
to hold all the argparser arguments, removing the need to define them in
the function signature itself.

This is useful especially on commands with lots of argument flags, as
otherwise you have to make sure you define all your arguments in just
the right order, which is very error prone.

By default, argparse will print errors, usage and help text directly to
stdout/stderr. It will also raise SystemExit to make the program stop
with an appropriate error code.

While this is appropriate for a CLI program, it is horrible for Err. The
SystemExit causes some strange behaviour where it doesn't make the bot
exit, but does prevent some processing (!restart no longer works, for
example) and the printing to stdout/stderr means you get no feedback
when issuing commands with incorrect arguments.

With these changes, all this output is nicely returned to the user,
instead.

(Bonus: The command name is used in the usage as well, instead of the
default `sys.arv[0]` which resulted in one of `err.py`, `errbot` or
`run_tests.py`.)
Some clients (looking at you, Telegram) convert certain ASCII character
combinations to nicer-looking unicode representations, which break the
argument parser. Account for this and undo the conversion to avoid user
frustration.
With this change it becomes possible to define a single `args` argument
to hold all the argparser arguments, removing the need to define them in
the function signature itself.

Full backward-compatibility is maintained as the default behavior
remains unchanged.
@zoni
Copy link
Member Author

zoni commented Oct 26, 2015

Preview of the usage/help text on Telegram (which removes blank lines for some reason):

2015-10-26 15 03 37 selection 001

gbin added a commit that referenced this pull request Oct 26, 2015
@gbin gbin merged commit f6e6f9d into errbotio:master Oct 26, 2015
@zoni zoni deleted the arg_botcmd-improvements branch October 26, 2015 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants