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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Aug 3, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gnovm/cmd/gno/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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 👍

},
c,
func(_ context.Context, args []string) error {
Expand Down
2 changes: 2 additions & 0 deletions tm2/pkg/commands/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

}

if len(c.subcommands) > 0 {
Expand Down
84 changes: 71 additions & 13 deletions tm2/pkg/commands/commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,17 +420,21 @@ func TestHelpUsage(t *testing.T) {
var buf bytes.Buffer
fs.SetOutput(&buf)

command := &Command{
name: "TestHelpUsage",
shortUsage: "TestHelpUsage [flags] <args>",
shortHelp: "Some short help.",
longHelp: "Some long help.",
flagSet: fs,
}

err := command.ParseAndRun(context.Background(), []string{"-h"})
assert.ErrorIs(t, err, flag.ErrHelp)
expectedOutput := strings.TrimSpace(`
tests := []struct {
name string
command *Command
expectedOutput string
}{
{
name: "normal case",
command: &Command{
name: "TestHelpUsage",
shortUsage: "TestHelpUsage [flags] <args>",
shortHelp: "Some short help",
longHelp: "Some long help.",
flagSet: fs,
},
expectedOutput: strings.TrimSpace(`
USAGE
TestHelpUsage [flags] <args>

Expand All @@ -443,8 +447,62 @@ FLAGS
-i 0 int
-s ... string
-x ... collection of strings (repeatable)
`) + "\n\n"
assert.Equal(t, expectedOutput, buf.String())
`) + "\n\n",
},
{
name: "no long help",
command: &Command{
name: "TestHelpUsage",
shortUsage: "TestHelpUsage [flags] <args>",
shortHelp: "Some short help",
flagSet: fs,
},
expectedOutput: strings.TrimSpace(`
USAGE
TestHelpUsage [flags] <args>

Some short help.

FLAGS
-b=false bool
-d 0s time.Duration
-f 0 float64
-i 0 int
-s ... string
-x ... collection of strings (repeatable)
`) + "\n\n",
},
{
name: "no short and no long help",
command: &Command{
name: "TestHelpUsage",
shortUsage: "TestHelpUsage [flags] <args>",
flagSet: fs,
},
expectedOutput: strings.TrimSpace(`
USAGE
TestHelpUsage [flags] <args>

FLAGS
-b=false bool
-d 0s time.Duration
-f 0 float64
-i 0 int
-s ... string
-x ... collection of strings (repeatable)
`) + "\n\n",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
buf.Reset()

err := tt.command.ParseAndRun(context.Background(), []string{"-h"})

assert.ErrorIs(t, err, flag.ErrHelp)
assert.Equal(t, tt.expectedOutput, buf.String())
})
}
}

// Forked from peterbourgon/ff/ffcli
Expand Down
Loading