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

mbed: config: print detailed help in subcommand #865

Merged
merged 1 commit into from
Mar 8, 2019

Conversation

naveenkaje
Copy link
Contributor

Print detailed usage when unsupported arguments are supplied with mbed config subcommand

@naveenkaje
Copy link
Contributor Author

naveenkaje commented Mar 4, 2019

Previous and new behavior details

    Before this change
    $mbed config -random unsupported
    [mbed] Working path "/home/navkaj01/code/mbed-os-example-blinky" (program)
    [mbed] No default d set in program "mbed-os-example-blinky"

    With this change
    $ mbed config -random unsupported
    [mbed] Working path "/home/navkaj01/code/mbed-os-example-blinky" (program)
    [mbed] No default unsupported set in program "mbed-os-example-blinky"
    usage: mbed config [-h] [-G] [-U] [-L] [-v] [-vv] [var] [value]

    Gets, sets or unsets mbed tool configuration options.
    Options can be global (via the --global switch) or local (per program)
    Global options are always overridden by local/program options.
    Currently supported options: target, toolchain, protocol, depth, cache, profile

    positional arguments:
      var                  Variable name. E.g. "target", "toolchain", "protocol"
      value                Value. Will show the currently set default value for a
                           variable if not specified.

    optional arguments:
      -h, --help           show this help message and exit
      -G, --global         Use global settings, not local
      -U, --unset          Unset the specified variable.
      -L, --list           List mbed tool configuration. Not to be confused with
                           compile configuration, e.g. "mbed compile --config".
      -v, --verbose        Verbose diagnostic output
      -vv, --very_verbose  Very verbose diagnostic output

    Before
    $ mbed config -random
    [mbed] Working path "/home/navkaj01/code/mbed-os-example-blinky" (program)
    usage: mbed config [-h] [-G] [-U] [-L] [-v] [-vv] [var] [value]
    mbed config: error: too few arguments

    With this change
    $ mbed config -random 
    [mbed] Working path "/home/navkaj01/code/mbed-os-example-blinky" (program)
    usage: mbed config [-h] [-G] [-U] [-L] [-v] [-vv] [var] [value]

   Gets, sets or unsets mbed tool configuration options.
   Options can be global (via the --global switch) or local (per program)
   Global options are always overridden by local/program options.
   Currently supported options: target, toolchain, protocol, depth, cache, profile

   positional arguments:
    var                  Variable name. E.g. "target", "toolchain", "protocol"
    value                Value. Will show the currently set default value for a
                       variable if not specified.

   optional arguments:
    -h, --help           show this help message and exit
    -G, --global         Use global settings, not local
    -U, --unset          Unset the specified variable.
    -L, --list           List mbed tool configuration. Not to be confused with
                       compile configuration, e.g. "mbed compile --config".
    -v, --verbose        Verbose diagnostic output
    -vv, --very_verbose  Very verbose diagnostic output
   usage: mbed config [-h] [-G] [-U] [-L] [-v] [-vv] [var] [value]
   mbed config: error: too few arguments

mbed/mbed.py Outdated
@@ -3250,7 +3250,9 @@ def config_(var=None, value=None, global_cfg=False, unset=False, list_config=Fal
else:
value = program.get_cfg(var)
action(('%s' % value) if value else 'No default %s set in program "%s"' % (name, program.name))
subcommands['config'].print_help()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we should exit() here?

@naveenkaje
Copy link
Contributor Author

@bridadan @theotherjimmy Please review

Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The supplied output seems really verbose. It's pretty easy to loose the actual error that occurred.

Is the concern that the current error output isn't helpful/accurate when a variable that has no effect is supplied to the mbed config command?

@TacoGrandeTX
Copy link

TacoGrandeTX commented Mar 5, 2019

We also wanted to fix this list:

Currently supported options: target, toolchain, protocol, depth, cache, profile

Tool paths are supported... and @theotherjimmy also mentioned colors

I think the other goal was to reject variables that weren't legit.

@bridadan
Copy link
Contributor

bridadan commented Mar 5, 2019

So just to be extra clear, fix the documentation about values are able to configured, and also error if an unsupported value is supplied?

@TacoGrandeTX
Copy link

@bridadan Yes - sorry for the confusion! @theotherjimmy corrected the wording of the Issue here: #862 (comment)

@naveenkaje
Copy link
Contributor Author

naveenkaje commented Mar 6, 2019

Old

$ mbed config -unsupported
usage: mbed config [-h] [-G] [-U] [-L] [-v] [-vv] [var] [value]
mbed config: error: too few arguments
$ mbed config -unsupported blah
[mbed] No default blah set in program "mbed-os-example-blinky"

New

$ mbed config -unsupported
[mbed] Working path "/home/navkaj01/code/mbed-os-example-blinky" (program)
usage: mbed config [-h] [-G] [-U] [-L] [-v] [-vv] [var] [value]
mbed config: error: Too few arguments. Run with -h for detailed help
---

$ mbed config -unsupported blah
[mbed] Working path "/home/navkaj01/code/mbed-os-example-blinky" (program)
[mbed] No default blah set in program "mbed-os-example-blinky"
[mbed] ERROR: run with -h for detailed help
---

@naveenkaje
Copy link
Contributor Author

Todo: colors

@bridadan
Copy link
Contributor

bridadan commented Mar 6, 2019

@naveenkaje colors can probably be in a separate PRs right? I don't think we do colors at all in mbed cli.

@naveenkaje
Copy link
Contributor Author

@naveenkaje colors can probably be in a separate PRs right? I don't think we do colors at all in mbed cli.

Sounds good to me

@theotherjimmy
Copy link
Contributor

@bridadan I think you may have misinterpreted "colors". @naveenkaje Is probably talking about adding the COLOR config option to the list. I think that would be part of this PR.

Print detailed usage when unsupported arguments are supplied
with mbed config subcommand
@theotherjimmy theotherjimmy merged commit 6377f75 into ARMmbed:master Mar 8, 2019
@theotherjimmy theotherjimmy mentioned this pull request Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants