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

Handle linebreaks in custom completions. #1162

Merged
merged 1 commit into from
Sep 9, 2020

Conversation

Luap99
Copy link
Contributor

@Luap99 Luap99 commented Jul 12, 2020

If a command/flag description contains a linebreak than the shell completion script will interpret this as new command/flag.

To fix this we should only use the first line from the description in the output.

e.g. cmd.Flags().String("flag", "", "Description for flag\nlonger description")

@CLAassistant
Copy link

CLAassistant commented Jul 12, 2020

CLA assistant check
All committers have signed the CLA.

@Luap99
Copy link
Contributor Author

Luap99 commented Jul 12, 2020

@marckhouzam Please take a look.
I noticed this while testing #1146 because this also messed up the formatting of the bash descriptions.

@marckhouzam
Copy link
Collaborator

@Luap99 Interesting find! What do you think would be best for a user:

  1. only taking the first line of the description or
  2. escaping the newline and showing the entire description (if we can)

I'm asking because I wondered if truncating a description would remove useful information.
Do you have concrete examples of descriptions using a newline so we can judge what would be best?

@Luap99
Copy link
Contributor Author

Luap99 commented Jul 14, 2020

@marckhouzam Here are two examples I found in the podman project:
tmpdir flag
latest flag

I would argue that the first line should contain enough information to understand what a command/flag does. If someone wants the full information they should use the man pages or --help.

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.

I tested this and it works great. Just a couple of nits on the comments.
Thanks!

custom_completions.go Outdated Show resolved Hide resolved
@Luap99 Luap99 force-pushed the completion-trim-newline branch from ce7aad9 to 52f8696 Compare July 14, 2020 20:29
If a command/flag description contains
a linebreak then the shell completion script
will interpret this as new command/flag.

To fix this we only use the first line
from the description in the output.

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
@Luap99 Luap99 force-pushed the completion-trim-newline branch from 52f8696 to 86f532b Compare July 14, 2020 20:34
@marckhouzam
Copy link
Collaborator

LGTM
This will protect both fish and zsh completions from descriptions that have a line break in them.

/cc @jharshman

Copy link
Collaborator

@jpmcb jpmcb 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 this fix! LGTM 🎸

@jpmcb jpmcb merged commit 8a63648 into spf13:master Sep 9, 2020
@Luap99 Luap99 deleted the completion-trim-newline branch December 31, 2020 13:04
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.

4 participants