-
Notifications
You must be signed in to change notification settings - Fork 20.6k
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
cmd/geth, internal/flags, go.mod: colorize cli help, support env vars #28103
Conversation
|
||
case *DirectoryFlag: | ||
flag.EnvVars = append(flag.EnvVars, envvar) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is super ugly, good luck making it nicer :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will become much easier with cli.v3 I think, because it uses generics to define the flag types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could always use reflect here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meh :) Not worth the complexity for fixing 20 lines of code.
|
||
// Annotate flag categories with colors (private template, so need to | ||
// copy-paste the entire thing here...) | ||
cli.AppHelpTemplate = strings.ReplaceAll(cli.AppHelpTemplate, "{{template \"visibleFlagCategoryTemplate\" .}}", "{{range .VisibleFlagCategories}}\n {{if .Name}}\u001B[33m{{.Name}}\u001B[0m\n\n {{end}}{{$flglen := len .Flags}}{{range $i, $e := .Flags}}{{if eq (subtract $flglen $i) 1}}{{$e}}\n{{else}}{{$e}}\n {{end}}{{end}}{{end}}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, so I explictly tried to avoid overriding the help templates to make future updates of the library easier. They keep adding stuff upstream and it's good not to have it custom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, but they made teh damn subtemplate private.
|
Random observation, re priority order
👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
With the latest change
|
…ethereum#28103) * cmd/geth, internal/flags, go.mod: colorize cli help, support env vars * internal/flags: use stdout, not stderr for terminal detection
…env vars (ethereum#28103)" This reverts commit 953f796.
…env vars (ethereum#28103)" This reverts commit 953f796.
This PR does three things: