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

Default strings in help shouldn't be Go escaped #346

Open
johnSchnake opened this issue Mar 16, 2022 · 2 comments · May be fixed by #347
Open

Default strings in help shouldn't be Go escaped #346

johnSchnake opened this issue Mar 16, 2022 · 2 comments · May be fixed by #347

Comments

@johnSchnake
Copy link

From spf13/cobra#1396:

Just spent another minute looking at this and that I think it is caused from the pflag library here

pflag/flag.go

Line 733 in 85dd5c8

line += fmt.Sprintf(" (default %q)", flag.DefValue)

The %q is what causes it to be a go-escaped string rather than just using "%v"; go fmt package states that %q for strings as:

%q	a double-quoted string safely escaped with Go syntax

So I think we need to put a PR up for that. I'll create an issue there to track it.

Originally posted by @johnSchnake in spf13/cobra#1396 (comment)

johnSchnake added a commit to johnSchnake/pflag that referenced this issue Mar 16, 2022
When creating the help text for a flag, the default value shouldn't be
be excaped. It isn't clear to an end user that we would be escaping those values.

Instead we should be printing the actual value and letting the users decide
when/how to add escaping based on how they're utilzing it.

Fixes spf13#346

Signed-off-by: John Schnake <jschnake@vmware.com>
@memreflect
Copy link

This behavior comes from Go's standard flag package. I imagine it looks bad for people with path names as default values on Windows or with default values of some flag providing regexp functionality: Playground demo

The duplicate backslashes and escaped quote marks will not happen with a custom Value type whose Type() string method does not return "string". A problem arises when you still want to quote the string, but that would mean adding quotes to the default value. That means you must check whether the flag value was changed or not to determine whether to strip quotes or not when working with the flag value.

Or you can use an empty string as the default value so pflag will not print a default value. That way, you can add the "(default ...)" string to your usage message manually and check whether the flag value was changed to determine if a user passed an empty string as the argument or if you got the default value, and there is no need to strip quotes; you simply set the default value yourself after flag parsing has completed if a user did not already set the flag value.

%q handles newlines and other control characters by escaping them in a double quoted string; %v/%s prints them unescaped. This behavior is great for things like colored output and backslashes, but it is terrible in almost every other case. %#q is a better option as it prefers backquoted strings, yielding `ping\pong` in the linked issue, but if there are control characters in the string (other than tab—see strconv.CanBackquote), then you will still see escaped characters, including the "duplicate" backslashes.

One solution that would not break compatibility with goflag and is not terribly intrusive is introducing something like a DefValueTransformer func(string) string field. The way I'm imagining it, theFlag.DefValueTransformer(theFlag.DefValue) would be called when printing default values if the field is not nil, instead of passing theFlag.DefValue directly to fmt.Sprintf, and you can customize output however you like using the function you set DefValueTransformer to. The resulting string would be used exactly as returned with %s rather than quoted with %q, so you would need to add quotes yourself, if necessary, and deal with any control characters. Of course, if the flag was not seen, the untransformed default value would get returned when you request the flag's value.

@tomqwpl
Copy link

tomqwpl commented Apr 29, 2022

Add my vote for this. Getting complaints that the default values print out with doubled backslashes in our tool that uses pflag.

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 a pull request may close this issue.

3 participants