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

fix: hide subcommand from short help when no subcommands #202

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: hide subcommand from short help when no subcommands #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0de627
wip: show subcommand ShortHelp bug with test
olizilla Oct 22, 2020
8025471
fix: show subcommand info in short help if there are any
olizilla Oct 22, 2020
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
4 changes: 3 additions & 1 deletion cli/helptext.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,11 @@ const shortHelpFormat = `USAGE
{{end}}{{if .Subcommands}}
SUBCOMMANDS
{{.Subcommands}}
{{end}}{{if .MoreHelp}}
{{.Indent}}For more information about each command, use:
{{.Indent}}'{{.Path}} <subcmd> --help'
Comment on lines 113 to 114
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it at all confusing that we let people know explicitly that they can run --help, but not the short -h?

I suspect not but figured I'd check in.

{{end}}{{if .MoreHelp}}
{{.Indent}}for more information about this command, use:
Copy link
Contributor

Choose a reason for hiding this comment

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

lowercase f here intentional?

{{.Indent}}'{{.Path}} --help'
{{end}}
`

Expand Down
45 changes: 45 additions & 0 deletions cli/helptext_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cli

import (
"bytes"
"strings"
"testing"

Expand Down Expand Up @@ -48,3 +49,47 @@ func TestSynopsisGenerator(t *testing.T) {
t.Fatal("Synopsis should contain options finalizer")
}
}

func TestShortHelp(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also add a test that checks on subcommands? The two things I'd like to watch for are:

  1. I'd like some signal that these tests are still useful even if someone changes "For more information about each command" to be rephrased slightly. This could also be aided by checking a constant.
  2. Making sure that subcommands are printed

// ShortHelp behaves differently depending on whether the command is the root or not.
root := &cmds.Command{
Subcommands: map[string]*cmds.Command{
"ls": {
Helptext: cmds.HelpText{
ShortDescription: `
Displays the contents of an IPFS or IPNS object(s) at the given path.
`},
},
},
}
// Ask for the help text for the ls command which has no subcommands
path := []string{"ls"}
buf := new(bytes.Buffer)
ShortHelp("ipfs", root, path, buf)
helpText := buf.String()
t.Logf("Short help text: %s", helpText)
if strings.Contains(helpText, "For more information about each command") {
t.Fatal("ShortHelp should not contain subcommand info")
}
}

func TestLongHelp(t *testing.T) {
root := &cmds.Command{
Subcommands: map[string]*cmds.Command{
"ls": {
Helptext: cmds.HelpText{
ShortDescription: `
Displays the contents of an IPFS or IPNS object(s) at the given path.
`},
},
},
}
path := []string{"ls"}
buf := new(bytes.Buffer)
LongHelp("ipfs", root, path, buf)
helpText := buf.String()
t.Logf("Long help text: %s", helpText)
if strings.Contains(helpText, "For more information about each command") {
t.Fatal("LongHelp should not contain subcommand info")
}
}