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

chore(cmd): print shortHelp if longHelp is empty #1011

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

tbruyelle
Copy link
Contributor

Currenlty, shortHelp is used when the command is listed from its parent command, and longHelp is used when the command is executed with its help flag.

What I found a little annoying is if longHelp is not provided, which is the case for the majority of commands, there's no text at all when the command is invoked with the help flag.

This change ensures the shortHelp is printed if longHelp is empty.

Example with `gno test` :

Before the change:

$ gno test -h
USAGE
  test [flags] <package> [<package>...]

FLAGS
  -precompile=false             precompile gno to go before testing
  -print-runtime-metrics=false  print runtime metrics (gas, memory, cpu cycles)
  -root-dir ...                 clone location of github.com/gnolang/gno (gnodev tries to guess it)
  -run ...                      test name filtering pattern
  -timeout 0s                   max execution time
  -update-golden-tests=false    writes actual as wanted in test comments
  -verbose=false                verbose output when running
  -with-native-fallback=false   use stdlibs/* if present, otherwise use supported native Go packages

error parsing commandline arguments: flag: help requested

After the change:

$ gno test -h
USAGE
  test [flags] <package> [<package>...]

Runs the tests for the specified packages.

FLAGS
  -precompile=false             precompile gno to go before testing
  -print-runtime-metrics=false  print runtime metrics (gas, memory, cpu cycles)
  -root-dir ...                 clone location of github.com/gnolang/gno (gnodev tries to guess it)
  -run ...                      test name filtering pattern
  -timeout 0s                   max execution time
  -update-golden-tests=false    writes actual as wanted in test comments
  -verbose=false                verbose output when running
  -with-native-fallback=false   use stdlibs/* if present, otherwise use supported native Go packages

error parsing commandline arguments: flag: help requested
Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@tbruyelle tbruyelle requested review from jaekwon, moul and a team as code owners August 1, 2023 09:19
@github-actions github-actions bot added 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related labels Aug 1, 2023
@@ -28,7 +28,7 @@ func newDocCmd(io *commands.IO) *commands.Command {
commands.Metadata{
Name: "doc",
ShortUsage: "doc [flags] <pkgsym>",
ShortHelp: "get documentation for the specified package or symbol (type, function, method, or variable/constant).",
ShortHelp: "get documentation for the specified package or symbol (type, function, method, or variable/constant)",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ShortHelp shouldn't end with a dot. Eventually, a dot is added if ShortHelp is used when the command is executed with its help flag.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to let the point in the definition and have a more basic fprintf in command.go.

Copy link
Contributor Author

@tbruyelle tbruyelle Aug 1, 2023

Choose a reason for hiding this comment

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

Well, this ShortHelp is the only one that had a final dot, all others haven't.

In addition, if we add dots at the end of ShortHelp, this will result in this when listing sub-commands :

$ gno
USAGE
  <subcommand> [flags] [<arg>...]

Runs the gno development toolkit

SUBCOMMANDS
  mod         Manage gno.mod.
  test        Runs the tests for the specified packages.
  lint        Runs the linter for the specified packages.
  run         Runs the specified gno files.
  build       Builds the specified gno package.
  precompile  Precompiles .gno files to .go.
  clean       Removes generated files and cached data.
  repl        Starts a GnoVM REPL.
  doc         get documentation for the specified package or symbol (type, function, method, or variable/constant).

IMO I prefer without the final dot.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, agree 👍

Currenlty, `shortHelp` is used when the command is listed from its
parent command, and `longHelp` is used when the command is executed with
its help flag.

What I found a little annoying is if `longHelp` is not provided, which
is the case for the majority of commands, there's no text at all when
the command is invoked with the help flag.

This change ensures the `shortHelp` is printed if `longHelp` is empty.
@@ -252,6 +252,8 @@ func usage(c *Command) string {

if c.longHelp != "" {
fmt.Fprintf(&b, "%s\n\n", c.longHelp)
} else if c.shortHelp != "" {
fmt.Fprintf(&b, "%s.\n\n", c.shortHelp)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fmt.Fprintf(&b, "%s.\n\n", c.shortHelp)
fmt.Fprintf(&b, "%s\n\n", c.shortHelp)

@moul moul merged commit 1fc25f9 into gnolang:master Aug 3, 2023
68 checks passed
@tbruyelle tbruyelle deleted the tbruyelle/chore/cmd-help branch August 10, 2023 11:33
Doozers pushed a commit to Doozers/gno that referenced this pull request Aug 31, 2023
@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants