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

new HelpOption: Allow full subcommand flag display #306

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
31 changes: 31 additions & 0 deletions _examples/shell/optionalflags/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package main

import (
"github.com/alecthomas/kong"
)

var cli struct {
DryRun string `help:"optional dry run flag"`
Mandatory bool `required:"" help:"mandatory global flag"`
Resource struct {
Create struct {
Name string `required:"" help:"name of the resource"`
} `cmd:"" help:"create a resource"`
Delete struct {
Scope string `required:"" help:"mandatory subcommand flag"`
Labels []string `help:"labels to match when deleting"`
Version string `help:"version to match when deleting"`
} `cmd:"" help:"delete resource(s)"`
} `cmd:"" help:"operate on resources"`
}

func main() {
ctx := kong.Parse(&cli,
kong.Name("help"),
kong.Description("An app demonstrating SubcommandsWithOptionalFlags"),
kong.UsageOnError(),
kong.ConfigureHelp(kong.HelpOptions{
SubcommandsWithOptionalFlags: true,
}))
ctx.Run()
}
1 change: 1 addition & 0 deletions build.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ func buildField(k *Kong, node *Node, v reflect.Value, ft reflect.StructField, fv
Mapper: mapper,
Tag: tag,
Target: fv,
Parent: node,
Enum: tag.Enum,
Passthrough: tag.Passthrough,

Expand Down
7 changes: 6 additions & 1 deletion help.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ type HelpOptions struct {
// Don't show the help associated with subcommands
NoExpandSubcommands bool

// Whether to include optional flags in command summaries
// If true, command summaries will include flags not marked as required
// If false (the default), command summaries will only show required flags
SubcommandsWithOptionalFlags bool

// Clamp the help wrap width to a value smaller than the terminal width.
// If this is set to a non-positive number, the terminal width is used; otherwise,
// the min of this value or the terminal width is used.
Expand Down Expand Up @@ -356,7 +361,7 @@ func collectCommandGroups(nodes []*Node) []helpCommandGroup {
}

func printCommandSummary(w *helpWriter, cmd *Command) {
w.Print(cmd.Summary())
w.Print(cmd.SummaryWithOptions(w.HelpOptions.SubcommandsWithOptionalFlags))
if cmd.Help != "" {
w.Indent().Wrap(cmd.Help)
}
Expand Down
57 changes: 57 additions & 0 deletions help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,63 @@ Commands:
})
}

// Tests HelpOptions.SubcommandsWithOptionalFlags
//
// MUST retain original behaviour by default (see "default" sub test)
// MUST show optional subcommand flags when SubcommandsWithOptionalFlags
//
// is enabled (see "forced" sub test)
//
// MUST show any _required_ parent flags on subcommands
// MUST NOT show any optional parent flags on subcommands
func TestSubcommandsWithOptionalFlags(t *testing.T) {
var cli struct {
Sub struct {
ParentFlag string `help:"I should only appear on parent"`
Mandatory string `required:"" help:"I should appear on all sub commands"`
MoreSub struct {
Flag string `help:"A flag."`
Required string `required:"" help:"A required flag."`
} `cmd help:"more sub help"`
Another struct {
} `cmd help:"another help"`
} `cmd help:"sub help"`
}
w := bytes.NewBuffer(nil)

app := mustNew(t, &cli,
kong.Name("test-app"),
kong.Description("A test app."),
kong.Writers(w, w),
kong.Exit(func(int) {}),
)

// Default: don't show flags on nested items unless they are `required`
t.Run("default", func(t *testing.T) {
expected := "Usage: test-app <command>\n\nA test app.\n\nFlags:\n -h, --help Show context-sensitive help.\n\nCommands:\n sub more-sub --mandatory=STRING --required=STRING\n more sub help\n\n sub another --mandatory=STRING\n another help\n\nRun \"test-app <command> --help\" for more information on a command.\n"
_, _ = app.Parse([]string{"--help"})
assert.Equal(t, expected, w.String())
})

w.Truncate(0)
app = mustNew(t, &cli,
kong.Name("test-app"),
kong.Description("A test app."),
kong.Writers(w, w),
kong.HelpOptions{
SubcommandsWithOptionalFlags: true,
},
kong.Exit(func(int) {}),
)

// Force flag display: don't show flags on nested items unless they are `required`
t.Run("forced", func(t *testing.T) {
expected := "Usage: test-app <command>\n\nA test app.\n\nFlags:\n -h, --help Show context-sensitive help.\n\nCommands:\n sub more-sub --mandatory=STRING --required=STRING [--flag=STRING]\n more sub help\n\n sub another --mandatory=STRING\n another help\n\nRun \"test-app <command> --help\" for more information on a command.\n"
_, _ = app.Parse([]string{"--help"})
assert.Equal(t, expected, w.String())
})
}

func TestHelpCompactNoExpand(t *testing.T) {
var cli struct {
One struct {
Expand Down
53 changes: 50 additions & 3 deletions model.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,25 @@ func (n *Node) Depth() int {
return depth
}

// Summary help string for the node (not including application name).
func (n *Node) Summary() string {
// SummaryWithOptions help string for the node (not including application name)
//
// Used both by help writing as well as other parts of the application
//
// Most cases just use Summary() (default settings)
//
// The help printer uses this optional version to pass options along.
//
// if includeOptionalFlags is true, all flags for a command will
// be included. see HelpOptions.IncludeOptionalFlagsInSummary
func (n *Node) SummaryWithOptions(includeOptionalFlags bool) string {
summary := n.Path()
if flags := n.FlagSummary(true); flags != "" {
var flags string
if includeOptionalFlags {
flags = n.FlagSummaryWithOptions(true)
} else {
flags = n.FlagSummary(true)
}
if flags != "" {
summary += " " + flags
}
args := []string{}
Expand Down Expand Up @@ -175,6 +190,12 @@ func (n *Node) Summary() string {
return summary
}

// Summary help string for the node (not including application name).
// Default behaviour used throughout the application
func (n *Node) Summary() string {
return n.SummaryWithOptions(false)
}

// FlagSummary for the node.
func (n *Node) FlagSummary(hide bool) string {
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry for the epically late response. Overall the change looks good, but I can't merge this as it's a breaking API change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the epically late response

All good! my turn this time, was on a short break :)

I can't merge this as it's a breaking API change.

FlagSummary is a fairly short function - so if you don't mind my duplicating it I could turn this into an additive change, ie.

  • Additive changes:
    • add Node.FlagSummaryWithOptions(bool, bool)
    • Node.SummaryWithOptions() calls Node.FlagSummaryWithOptions(bool, bool) instead
  • Reverted:
    • Node.FlagSummary() returns to its original arity
    • Node.Summary() goes back to calling Node.FlagSummary(bool)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually - given the current PR is unacceptable, I'll just make the changes now and see what you think :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rebased, reworked changes to avoid API modification (and attempting to minimise duplication where possible)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just extra explanation re. bonus change in b196f4d : appears that my rebase picked up some change which is invalid with the current golangci-lint (see previous execution here: https://github.com/alecthomas/kong/actions/runs/3126674306/jobs/5072442276)

Seem fine now

required := []string{}
Expand All @@ -190,6 +211,31 @@ func (n *Node) FlagSummary(hide bool) string {
return strings.Join(required, " ")
}

// FlagSummaryWithOptions is the same behaviour as FlagSummary but
// also includes any optional flags associated with the Node
func (n *Node) FlagSummaryWithOptions(hide bool) string {
required := []string{}
optional := []string{}
count := 0
for _, group := range n.AllFlags(hide) {
for _, flag := range group {
count++
// Show required flags
if flag.Required {
required = append(required, flag.Summary())
} else if flag.Parent == n {
// Show optional flags _if they belong to us_
optional = append(optional, flag.Summary())
}
}
}
if len(optional) > 0 {
// Group optional flags together and surround with brackets
required = append(required, fmt.Sprintf("[%s]", strings.Join(optional, " ")))
}
return strings.Join(required, " ")
}

// FullPath is like Path() but includes the Application root node.
func (n *Node) FullPath() string {
root := n
Expand Down Expand Up @@ -250,6 +296,7 @@ type Value struct {
Mapper Mapper
Tag *Tag
Target reflect.Value
Parent *Node
Required bool
Set bool // Set to true when this value is set through some mechanism.
Format string // Formatting directive, if applicable.
Expand Down
Loading