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

Hide Style Flags consistently #457

Merged
merged 2 commits into from
Nov 28, 2023
Merged

Hide Style Flags consistently #457

merged 2 commits into from
Nov 28, 2023

Conversation

maaslalani
Copy link
Contributor

@maaslalani maaslalani commented Nov 28, 2023

Prevents style flags from leaking through when a user makes an error in the usage.

For example:

gum write --help

using the --help flag produces a different usage than the user making an error with the flags.

gum write --error

This is because we perform the hiding of the flags on BeforeReset which is called for the help flag but not on an error.

The downside of this approach is that we must duplicate the style struct so that we can maintain a fully hidden embeddable struct while also having a non-hidden struct so the style command has proper usage / help / errors.

This can be resolve once the following issue is implemented:

alecthomas/kong#316

Copy link
Member

@aymanbagabas aymanbagabas left a comment

Choose a reason for hiding this comment

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

Instead of defining two style structs, why don't we mark the flags hidden by default and "unhide" them during the BeforeReset hook and when encountering a --help flag?

Copy link
Member

@aymanbagabas aymanbagabas left a comment

Choose a reason for hiding this comment

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

LGTM, I would add a comment referring to the issue url in the code

@maaslalani maaslalani merged commit 01a6651 into main Nov 28, 2023
8 of 9 checks passed
@maaslalani maaslalani deleted the hide-style-help branch November 28, 2023 19:17
@@ -74,7 +75,6 @@ func main() {
if errors.Is(err, exit.ErrAborted) {
os.Exit(exit.StatusAborted)
}
fmt.Println(err)

Choose a reason for hiding this comment

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

@maaslalani Why was this deleted? Now it's not printing any error.

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.

3 participants