Skip to content

mbed: config: print detailed help in subcommand#865

Merged
theotherjimmy merged 1 commit intoARMmbed:masterfrom
naveenkaje:fix_config_help
Mar 8, 2019
Merged

mbed: config: print detailed help in subcommand#865
theotherjimmy merged 1 commit intoARMmbed:masterfrom
naveenkaje:fix_config_help

Conversation

@naveenkaje
Copy link
Copy Markdown
Contributor

@naveenkaje naveenkaje commented Mar 4, 2019

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

@naveenkaje
Copy link
Copy Markdown
Contributor Author

naveenkaje commented Mar 4, 2019 •
edited
Loading

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

naveenkaje
naveenkaje commented Mar 4, 2019
View reviewed changes
mbed/mbed.py Outdated
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
Copy Markdown
Contributor Author

@naveenkaje naveenkaje Mar 4, 2019

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
Copy Markdown
Contributor Author

naveenkaje commented Mar 4, 2019

@bridadan @theotherjimmy Please review

@naveenkaje naveenkaje mentioned this pull request Mar 4, 2019
Closed
bridadan
bridadan reviewed Mar 5, 2019
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown

TacoGrandeTX commented Mar 5, 2019 •
edited
Loading

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
Copy Markdown
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
Copy Markdown

TacoGrandeTX commented Mar 6, 2019

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

@naveenkaje
Copy link
Copy Markdown
Contributor Author

naveenkaje commented Mar 6, 2019 •
edited
Loading

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
Copy Markdown
Contributor Author

naveenkaje commented Mar 6, 2019

Todo: colors

@bridadan
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

naveenkaje 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.

Sounds good to me

@theotherjimmy
Copy link
Copy Markdown
Contributor

theotherjimmy commented Mar 7, 2019

@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.

mbed: config: print detailed help in subcommand
449606f 
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
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@theotherjimmy theotherjimmy theotherjimmy approved these changes

@bridadan bridadan bridadan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@naveenkaje @TacoGrandeTX @bridadan @theotherjimmy