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

Fix --version help and output for plugins #2180

Merged
merged 3 commits into from
Oct 12, 2024
Merged

Conversation

nirs
Copy link
Contributor

@nirs nirs commented Aug 24, 2024

When writing a plugin and setting CommandDisplayNameAnnotation you want the display name to be used in help text. This did not work when setting command Version since the help text and version templates used the command Name().

This change makes command DisplayName() public, so it can be used in templates or other code that want to use this value, and fix --version help text and template to use the command DisplayName().

@nirs
Copy link
Contributor Author

nirs commented Aug 24, 2024

Lint errors are fixed in #2181, go 1.23 tests added in #2182. I rebased on both temporarily to verify all tests and go version with this change.

@github-actions github-actions bot added the area/docs-generation Generation of docs via Cobra label Aug 24, 2024
@github-actions github-actions bot added the area/github For changes to Github specific things not shipped in the library label Aug 24, 2024
@marckhouzam
Copy link
Collaborator

I’ll let you rebase this one first @nirs

nirs added 3 commits August 24, 2024 20:14
When setting Command.Version, a --version option is added. The help
message for the --version command did not consider the command display
name:

    Flags:
      -h, --help      help for kubectl plugin
      -v, --version   version for kubectl-plugin

With this change the help test is consistent with other flags:

    Flags:
      -h, --help      help for kubectl plugin
      -v, --version   version for kubectl plugin
This allows using the display name in templates or other code that want
to use the same value.
The version template used `{{.Name}}` but for plugins you want to use
`{{.DisplayName}}` to be consistent with other help output.

With this change will show:

    $ kubectl plugin --version
    kubectl plugin version 1.0.0

This may cause issues for programs processing the output if the program
assumes that the name of the program never contains spaces, and the
version string is the third word. Users can use their own template if
they think that this is an issue.

If we think that this can break users we can drop this change and let
users opt in by setting their own template using `{{.DisplayName}}`.
@nirs
Copy link
Contributor Author

nirs commented Aug 24, 2024

@marckhouzam can be merged now

@nirs
Copy link
Contributor Author

nirs commented Sep 7, 2024

@marckhouzam can you check gain?

@marckhouzam
Copy link
Collaborator

@nirs On the last commit you mention

    This may cause issues for programs processing the output if the program
    assumes that the name of the program never contains spaces, and the
    version string is the third word. Users can use their own template if
    they think that this is an issue.

    If we think that this can break users we can drop this change and let
    users opt in by setting their own template using `{{.DisplayName}}`.

I'm not concerned because this will only affect programs that use the CommandDisplayNameAnnotation annotation, in which case your change is correct.

Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

@marckhouzam marckhouzam merged commit 5bef9d8 into spf13:main Oct 12, 2024
18 checks passed
@marckhouzam marckhouzam added this to the 1.9.0 milestone Oct 12, 2024
@marckhouzam marckhouzam modified the milestones: 1.8.1, 1.9.0 Nov 4, 2024
ayax79 pushed a commit to ayax79/cobra that referenced this pull request Nov 28, 2024
* Fix --version help with CommandDisplayNameAnnotation

When setting Command.Version, a --version option is added. The help
message for the --version command did not consider the command display
name:

    Flags:
      -h, --help      help for kubectl plugin
      -v, --version   version for kubectl-plugin

With this change the help test is consistent with other flags:

    Flags:
      -h, --help      help for kubectl plugin
      -v, --version   version for kubectl plugin

* Make command DisplayName() public

This allows using the display name in templates or other code that want
to use the same value.

* Use display name in version template

The version template used `{{.Name}}` but for plugins you want to use
`{{.DisplayName}}` to be consistent with other help output.

With this change will show:

    $ kubectl plugin --version
    kubectl plugin version 1.0.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs-generation Generation of docs via Cobra area/github For changes to Github specific things not shipped in the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants