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

Adjust sub-command template to properly output sub-sub-command placeholder. #1683

Merged
merged 3 commits into from
Feb 21, 2023
Merged

Adjust sub-command template to properly output sub-sub-command placeholder. #1683

merged 3 commits into from
Feb 21, 2023

Conversation

aryounce
Copy link
Contributor

What type of PR is this?

  • bug
  • cleanup

What this PR does / why we need it:

  • template.go includes an improper conditional in the sub-command help text template that relies on flag visibility instead command visibility for the Command instance.

Which issue(s) this PR fixes:

Fixes #1682

Release Notes

Fix sub-command help output to exclude sub-sub-command placeholder when command has no sub-commands.

@aryounce aryounce requested a review from a team as a code owner February 16, 2023 00:47
@avorima
Copy link
Contributor

avorima commented Feb 16, 2023

I played around with this a bit and did not get it to produce different output than current main. Could you provide an example?

dearchap
dearchap previously approved these changes Feb 18, 2023
Copy link
Contributor

@dearchap dearchap left a comment

Choose a reason for hiding this comment

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

This may not be sufficient since Help Command is added by default. So it really wont change any behaviour

@aryounce
Copy link
Contributor Author

@dearchap This is for when the help command is disabled. Then there are zero sub-commands (which is the use case where I encountered the problem).

@dearchap
Copy link
Contributor

That makes sense. Can you see why the checks are failing ? You might have to run make v2approve or make v3approve

@aryounce
Copy link
Contributor Author

@dearchap Running make diffcheck (which is failing in the tests) locally succeeds. Running make v2approve and make v3approve don't appear to have any affect. I don't have the ability to re-run the Github actions and thought that perhaps the encountered error is transient. Advice on how to proceed would be greatly appreciated.

@avorima
Copy link
Contributor

avorima commented Feb 20, 2023

@aryounce I checked the output with the following program and there was no change from main to your PR. Maybe my example is incorrect?

package main

import "github.com/urfave/cli/v3"

func main() {
	app := &cli.App{
		Name: "test",
		Commands: []*cli.Command{
			{
				Name: "sub",
				Commands: []*cli.Command{
					{Name: "sub-sub", Hidden: true},
				},
				HideHelp: true,
			},
		},
	}
	_ = app.Run([]string{"test", "sub"})
}

@avorima
Copy link
Contributor

avorima commented Feb 20, 2023

You need to run make generate

@aryounce
Copy link
Contributor Author

aryounce commented Feb 20, 2023

@avorima I've modified your example to demonstrate the issue I'm attempting to address. Be sure to run this outside of my branch (or off of main) to prevent my changes having an effect. My version of your code is:

package main

import (
	"fmt"
	"os"
	"github.com/urfave/cli/v3"
)

func main() {
	app := &cli.App{
		Name: "test",
		Commands: []*cli.Command{
			{
				Name: "default",
				Action: dumpCmdInfo,
			},
			{
				Name: "hide-help",
				Action: dumpCmdInfo,
				HideHelp: true,
			},
			{
				Name: "hide-help-cmd",
				Action: dumpCmdInfo,
				HideHelpCommand: true,
			},
			{
				Name: "hidden",
				Action: dumpCmdInfo,
				Hidden: true,
			},
		},
	}
	app.Run(os.Args)
}

func dumpCmdInfo(c *cli.Context) error {
	fmt.Println(c.Command.Name)
	fmt.Printf("  VisibleCommands() = %+v\n", c.Command.VisibleCommands())
	fmt.Printf("  VisibleFlags() = %+v\n", c.Command.VisibleFlags())
	return nil
}

default

When running the default sub-command (which has no sub-commands of its own) the output is as follows. Notice that the help text only shows [command options] under USAGE. This appears to use the CommandHelpTemplate: https://github.com/urfave/cli/blob/main/template.go#L59-L67

Help Output

NAME:
   test default

USAGE:
   test default [command options] [arguments...]

OPTIONS:
   --help, -h  show help (default: false)

Command Info Dump

default
  VisibleCommands() = [0x105159b40]
  VisibleFlags() = [--help, -h	show help (default: false)]

hidden

This uses CommandHelpTemplate to form its USAGE section, like default.

Help Output

NAME:
   test hidden

USAGE:
   test hidden [command options] [arguments...]

OPTIONS:
   --help, -h  show help (default: false)

Command Info Dump

hidden
  VisibleCommands() = [0x104471b40]
  VisibleFlags() = [--help, -h	show help (default: false)]

hide-help

Has no help output because the HideHelp property on Command is set to false.

Command Info Dump

hide-help
  VisibleCommands() = []
  VisibleFlags() = []

hide-help-cmd

This is the use case my PR is attempting to address. Although it has no sub-commands the help text output contains command [command options] which implies that it does have one or more sub-commands. This is because it (currently) uses SubcommandHelpTemplate, https://github.com/urfave/cli/blob/main/template.go#L79-L87, which relies upon whether or not VisibleFlags() returns a zero-value.

Help Output

NAME:
   test hide-help-cmd

USAGE:
   test hide-help-cmd command [command options] [arguments...]

OPTIONS:
   --help, -h  show help (default: false)

Command Info Dump

hide-help-cmd
  VisibleCommands() = []
  VisibleFlags() = [--help, -h	show help (default: false)]

@avorima
Copy link
Contributor

avorima commented Feb 21, 2023

Thank you, I can see the difference now.

@dearchap dearchap merged commit 6e70b03 into urfave:main Feb 21, 2023
@aryounce aryounce deleted the ARY/Subcommand-Template-Fix branch February 21, 2023 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sub-command help template shows command [command options] inappropriately
3 participants