Skip to content

Commit

Permalink
Fix:(issue_557) Make help output consistent between different invocat…
Browse files Browse the repository at this point in the history
…ions
  • Loading branch information
dearchap committed Sep 21, 2022
1 parent 0ee87b4 commit e925d26
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 35 deletions.
5 changes: 4 additions & 1 deletion app_test.go
Expand Up @@ -175,10 +175,13 @@ func ExampleApp_Run_commandHelp() {
// greet describeit - use it to see a description
//
// USAGE:
// greet describeit [arguments...]
// greet describeit [command options] [arguments...]
//
// DESCRIPTION:
// This is how we describe describeit the function
//
// OPTIONS:
// --help, -h show help (default: false)
}

func ExampleApp_Run_noAction() {
Expand Down
50 changes: 47 additions & 3 deletions command.go
Expand Up @@ -62,6 +62,9 @@ type Command struct {
// cli.go uses text/template to render templates. You can
// render custom help text by setting this variable.
CustomHelpTemplate string

// categories contains the categorized commands and is populated on app startup
categories CommandCategories
}

type Commands []*Command
Expand Down Expand Up @@ -166,7 +169,7 @@ func (c *Command) Run(ctx *Context) (err error) {
}

if c.Action == nil {
c.Action = helpSubcommand.Action
c.Action = helpCommand.Action
}

cCtx.Command = c
Expand Down Expand Up @@ -280,7 +283,7 @@ func (c *Command) startApp(ctx *Context) error {
if c.Action != nil {
app.Action = c.Action
} else {
app.Action = helpSubcommand.Action
app.Action = helpCommand.Action
}
app.OnUsageError = c.OnUsageError

Expand All @@ -294,7 +297,12 @@ func (c *Command) startApp(ctx *Context) error {
// VisibleFlagCategories returns a slice containing all the visible flag categories with the flags they contain
func (c *Command) VisibleFlagCategories() []VisibleFlagCategory {
if c.flagCategories == nil {
return []VisibleFlagCategory{}
c.flagCategories = newFlagCategories()
for _, fl := range c.Flags {
if cf, ok := fl.(CategorizableFlag); ok {
c.flagCategories.AddFlag(cf.GetCategory(), cf)
}
}
}
return c.flagCategories.VisibleCategories()
}
Expand All @@ -304,6 +312,42 @@ func (c *Command) VisibleFlags() []Flag {
return visibleFlags(c.Flags)
}

// VisibleCategories returns a slice of categories and commands that are
// Hidden=false
func (c *Command) VisibleCategories() []CommandCategory {
ret := []CommandCategory{}
if c.categories == nil {
c.categories = newCommandCategories()
for _, command := range c.Subcommands {
c.categories.AddCommand(command.Category, command)
}
sort.Sort(c.categories.(*commandCategories))
}
for _, category := range c.categories.Categories() {
if visible := func() CommandCategory {
if len(category.VisibleCommands()) > 0 {
return category
}
return nil
}(); visible != nil {
ret = append(ret, visible)
}
}

return ret
}

// VisibleCommands returns a slice of the Commands with Hidden=false
func (c *Command) VisibleCommands() []*Command {
var ret []*Command
for _, command := range c.Subcommands {
if !command.Hidden {
ret = append(ret, command)
}
}
return ret
}

func (c *Command) appendFlag(fl Flag) {
if !hasFlag(c.Flags, fl) {
c.Flags = append(c.Flags, fl)
Expand Down
17 changes: 11 additions & 6 deletions godoc-current.txt
Expand Up @@ -82,12 +82,10 @@ DESCRIPTION:

OPTIONS:{{range .VisibleFlagCategories}}
{{if .Name}}{{.Name}}
{{end}}{{range .Flags}}{{.}}
{{end}}{{end}}{{else}}{{if .VisibleFlags}}
{{end}}{{range .Flags}}{{.}}{{end}}{{end}}{{else}}{{if .VisibleFlags}}

OPTIONS:
{{range .VisibleFlags}}{{.}}
{{end}}{{end}}{{end}}
{{range .VisibleFlags}}{{.}}{{end}}{{end}}{{end}}

This comment has been minimized.

Copy link
@isbm

isbm Oct 6, 2022

That actually breaks the help listing, rendering all the options/flags into one continuous line like so:

OPTIONS:
   --arch value, -a value                Set architecture for the system root. Choices: aarch64, arm, mips, mips64, mipsn32, x86_64.--create, -c  Create a new system root (default: false)--delete, -d            Delete a system root by name (default: false)--init, -i        Init default system root (default: false)--list, -l  List available system roots
 (default: false)--name value, -n value  Set name of the system root--path, -p                                                                    Display path of an active system root (default: false)--set, -s  Set default system root by name (default: false)--verbose, -v  Show debugging log (default: false)

...while it expected to be like so:

OPTIONS:
  --arch value, -a value  Set architecture for the system root. Choices: aarch64, arm, mips, mips64, mipsn32, x86_64.
  --create, -c            Create a new system root (default: false)
  --delete, -d            Delete a system root by name (default: false)
  --init, -i              Init default system root (default: false)
  --list, -l              List available system roots (default: false)
  --name value, -n value  Set name of the system root
  --path, -p              Display path of an active system root (default: false)
  --set, -s               Set default system root by name (default: false)
  --verbose, -v           Show debugging log (default: false)

To fix that, I had to copy your default template, override and undo your change.

`
CommandHelpTemplate is the text template for the command help topic. cli.go
uses text/template to render templates. You can render custom help text by
Expand Down Expand Up @@ -161,8 +159,7 @@ COMMANDS:{{range .VisibleCategories}}{{if .Name}}
{{$s := join .Names ", "}}{{$s}}{{ $sp := subtract $cv (offset $s 3) }}{{ indent $sp ""}}{{wrap .Usage $cv}}{{end}}{{end}}{{end}}{{if .VisibleFlags}}

OPTIONS:
{{range .VisibleFlags}}{{.}}
{{end}}{{end}}
{{range .VisibleFlags}}{{.}}{{end}}{{end}}
`
SubcommandHelpTemplate is the text template for the subcommand help topic.
cli.go uses text/template to render templates. You can render custom help
Expand Down Expand Up @@ -558,6 +555,7 @@ type Command struct {
// cli.go uses text/template to render templates. You can
// render custom help text by setting this variable.
CustomHelpTemplate string

// Has unexported fields.
}
Command is a subcommand for a cli.App.
Expand All @@ -576,6 +574,13 @@ func (c *Command) Run(ctx *Context) (err error)
Run invokes the command given the context, parses ctx.Args() to generate
command-specific flags

func (c *Command) VisibleCategories() []CommandCategory
VisibleCategories returns a slice of categories and commands that are
Hidden=false

func (c *Command) VisibleCommands() []*Command
VisibleCommands returns a slice of the Commands with Hidden=false

func (c *Command) VisibleFlagCategories() []VisibleFlagCategory
VisibleFlagCategories returns a slice containing all the visible flag
categories with the flags they contain
Expand Down
64 changes: 50 additions & 14 deletions help.go
Expand Up @@ -15,33 +15,59 @@ const (
helpAlias = "h"
)

var helpCommand = &Command{
// this instance is to avoid recursion in the ShowCommandHelp which can
// add a help command again
var helpCommandDontUse = &Command{
Name: helpName,
Aliases: []string{helpAlias},
Usage: "Shows a list of commands or help for one command",
ArgsUsage: "[command]",
Action: func(cCtx *Context) error {
args := cCtx.Args()
if args.Present() {
return ShowCommandHelp(cCtx, args.First())
}

_ = ShowAppHelp(cCtx)
return nil
},
}

var helpSubcommand = &Command{
var helpCommand = &Command{
Name: helpName,
Aliases: []string{helpAlias},
Usage: "Shows a list of commands or help for one command",
ArgsUsage: "[command]",
Action: func(cCtx *Context) error {
args := cCtx.Args()
if args.Present() {
return ShowCommandHelp(cCtx, args.First())
argsPresent := args.First() != ""
firstArg := args.First()

// This action can be triggered by a "default" action of a command
// or via cmd.Run when cmd == helpCmd. So we have following possibilities
//
// 1 $ app
// 2 $ app help
// 3 $ app foo
// 4 $ app help foo
// 5 $ app foo help

// Case 4. when executing a help command set the context to parent
// to allow resolution of subsequent args. This will transform
// $ app help foo
// to
// $ app foo
// which will then be handled as case 3
if cCtx.Command.Name == helpName || cCtx.Command.Name == helpAlias {
cCtx = cCtx.parentContext
}

// Case 4. $ app hello foo
// foo is the command for which help needs to be shown
if argsPresent {
return ShowCommandHelp(cCtx, firstArg)
}

// Case 1 & 2
// Special case when running help on main app itself as opposed to indivdual
// commands/subcommands
if cCtx.parentContext.App == nil {
_ = ShowAppHelp(cCtx)
return nil
}

// Case 3, 5
return ShowSubcommandHelp(cCtx)
},
}
Expand Down Expand Up @@ -212,9 +238,19 @@ func ShowCommandHelp(ctx *Context, command string) error {

for _, c := range ctx.App.Commands {
if c.HasName(command) {
if !ctx.App.HideHelpCommand && !c.HasName(helpName) && len(c.Subcommands) != 0 {
c.Subcommands = append(c.Subcommands, helpCommandDontUse)
}
if !ctx.App.HideHelp && HelpFlag != nil {
c.appendFlag(HelpFlag)
}
templ := c.CustomHelpTemplate
if templ == "" {
templ = CommandHelpTemplate
if len(c.Subcommands) == 0 {
templ = CommandHelpTemplate
} else {
templ = SubcommandHelpTemplate
}
}

HelpPrinter(ctx.App.Writer, templ, c)
Expand Down
12 changes: 7 additions & 5 deletions help_test.go
Expand Up @@ -186,7 +186,7 @@ func Test_helpSubcommand_Action_ErrorIfNoTopic(t *testing.T) {

c := NewContext(app, set, nil)

err := helpSubcommand.Action(c)
err := helpCommand.Action(c)

if err == nil {
t.Fatalf("expected error from helpCommand.Action(), but got nil")
Expand Down Expand Up @@ -248,7 +248,7 @@ func TestShowCommandHelp_HelpPrinter(t *testing.T) {
fmt.Fprint(w, "yo")
},
command: "",
wantTemplate: SubcommandHelpTemplate,
wantTemplate: AppHelpTemplate,
wantOutput: "yo",
},
{
Expand Down Expand Up @@ -333,7 +333,7 @@ func TestShowCommandHelp_HelpPrinterCustom(t *testing.T) {
fmt.Fprint(w, "yo")
},
command: "",
wantTemplate: SubcommandHelpTemplate,
wantTemplate: AppHelpTemplate,
wantOutput: "yo",
},
{
Expand Down Expand Up @@ -1357,10 +1357,13 @@ DESCRIPTION:
and a description long
enough to wrap in this test
case
OPTIONS:
--help, -h show help (default: false)
`

if output.String() != expected {
t.Errorf("Unexpected wrapping, got:\n%s\nexpected: %s",
t.Errorf("Unexpected wrapping, got:\n%s\nexpected:\n%s",
output.String(), expected)
}
}
Expand Down Expand Up @@ -1426,7 +1429,6 @@ USAGE:
OPTIONS:
--help, -h show help (default: false)
`

if output.String() != expected {
Expand Down
9 changes: 3 additions & 6 deletions template.go
Expand Up @@ -54,12 +54,10 @@ DESCRIPTION:
OPTIONS:{{range .VisibleFlagCategories}}
{{if .Name}}{{.Name}}
{{end}}{{range .Flags}}{{.}}
{{end}}{{end}}{{else}}{{if .VisibleFlags}}
{{end}}{{range .Flags}}{{.}}{{end}}{{end}}{{else}}{{if .VisibleFlags}}
OPTIONS:
{{range .VisibleFlags}}{{.}}
{{end}}{{end}}{{end}}
{{range .VisibleFlags}}{{.}}{{end}}{{end}}{{end}}
`

// SubcommandHelpTemplate is the text template for the subcommand help topic.
Expand All @@ -80,8 +78,7 @@ COMMANDS:{{range .VisibleCategories}}{{if .Name}}
{{$s := join .Names ", "}}{{$s}}{{ $sp := subtract $cv (offset $s 3) }}{{ indent $sp ""}}{{wrap .Usage $cv}}{{end}}{{end}}{{end}}{{if .VisibleFlags}}
OPTIONS:
{{range .VisibleFlags}}{{.}}
{{end}}{{end}}
{{range .VisibleFlags}}{{.}}{{end}}{{end}}
`

var MarkdownDocTemplate = `{{if gt .SectionNum 0}}% {{ .App.Name }} {{ .SectionNum }}
Expand Down

0 comments on commit e925d26

Please sign in to comment.