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

x/tools/go/buildutil: split TagsFlagDoc into multiple lines? #23464

Open
mvdan opened this Issue Jan 17, 2018 · 9 comments

Comments

Projects
None yet
5 participants
@mvdan
Copy link
Member

mvdan commented Jan 17, 2018

What version of Go are you using (go version)?

go version devel +7e054553ad Tue Jan 16 15:11:05 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

Add a -tags flag to a static analysis tool of mine:

func init() {
        flag.Var((*buildutil.TagsFlag)(&build.Default.BuildTags), "tags", buildutil.TagsFlagDoc)
}

Then run mytool -h.

What did you expect to see?

The long flag usage line to be split in multiple lines somehow.

Usage of mytool:
  -tags build tags
        a list of build tags to consider satisfied during the build.
        For more information about build tags, see the description of
        build constraints in the documentation for the go/build package.

What did you see instead?

Usage of mytool:
  -tags build tags
        a list of build tags to consider satisfied during the build. For more information about build tags, see the description of build constraints in the documentation for the go/build package

I initially thought that changing the string constant would be enough, for example by adding "\n\t" twice to split the one log line into three lines.

However, the flag package is not consistent about placing the usage line, so I am not sure if that is enough:

// Boolean flags of one ASCII letter are so common we
// treat them specially, putting their usage on the same line.
if len(s) <= 4 { // space, space, '-', 'x'.
        s += "\t"
} else {
        // Four spaces before the tab triggers good alignment
        // for both 4- and 8-space tab stops.
        s += "\n    \t"
}

So perhaps this should be a proposal to add special handling of long flag usage texts in the flag package, to wrap them around a certain around of columns. But I imagine that such a feature wouldn't be welcome, as the package is meant to be simple, and one has multiple workarounds available like using shorter texts, using a custom Usage func, or using a third-party flag package.

Still, it seems to me like this is a problem with either buildutil or flag.

/cc @alandonovan @shurcooL

@dmitshur

This comment has been minimized.

Copy link
Member

dmitshur commented Jan 17, 2018

The long flag usage line to be split in multiple lines somehow.

Just checking, but doesn't your terminal perform automatic wrapping of long lines?

For me, I get something like this:

image

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Jan 17, 2018

Yes, but that's harder to read since there is no indentation. The end of that long line and the start of the next flag can be confused as one single paragraph. That's a good point that I didn't mention, though.

@dmitshur

This comment has been minimized.

Copy link
Member

dmitshur commented Feb 9, 2018

Splitting the help string into multiple lines is more viable in Go 1.10, given that https://tip.golang.org/doc/go1.10#flag was done:

PrintDefaults now adds appropriate indentation after newlines in flag usage strings, so that multi-line usage strings display nicely.

(Discovered via https://speakerdeck.com/campoy/the-state-of-go-1-dot-10?slide=23. /cc @campoy FYI.)

@mvdan

This comment has been minimized.

Copy link
Member Author

mvdan commented Feb 9, 2018

Yes - I saw Francesc's slides too, and the same thing came to mind. I forgot to follow up on this issue, though :)

Since the x repos tend to support the latest 2 Go releases, I think it would be okay to add the newlines once 1.10 is out. Those building with 1.9 and using that help string will get output that is slightly off, but I think that is okay.

The other option is waiting until 1.11 is out. I'm fine either way.

@spf13

This comment has been minimized.

Copy link
Contributor

spf13 commented Mar 26, 2018

ping @alandonovan for decision

@alandonovan

This comment has been minimized.

Copy link
Contributor

alandonovan commented Mar 27, 2018

If this problem really needs to be solved, its solution belongs in the flag package, not in every flag docstring, since the author of the flag docstring cannot know how long to make each line as this depends on the indentation applied by the flag package.

@dmitshur

This comment has been minimized.

Copy link
Member

dmitshur commented Mar 28, 2018

But the flag package can't reliably do wrapping with indentation either, not without knowing the width of the terminal where its output is being shown. Is that possible?

@rsc's decision in #20799 (which led to the Go 1.10 flag change 0828ec1, described at https://golang.org/doc/go1.10#flag) seemed to suggest that it's ok for the docstring to be split into multiple lines.

since the author of the flag docstring cannot know how long to make each line as this depends on the indentation applied by the flag package.

The indentation applied by the flag package is fixed and predictable. Since the docstrings are meant to be used with flag package, it's not completely unreasonable for their authors to know about the flag package's indentation. I don't mean it's the best solution, but it is viable. Both of the approaches are.

@alandonovan

This comment has been minimized.

Copy link
Contributor

alandonovan commented Mar 28, 2018

@dmitshur

This comment has been minimized.

Copy link
Member

dmitshur commented Mar 28, 2018

Logf does something much simpler: it inserts a number of tabs after each newline, so that on an infinitely wide terminal, multiline comments indent nicely.
[…]
Changing flag to resemble Logf seems fine.

Oh, but that’s already done. That’s exactly the 1.10 flag change I mentioned above. Sorry if I’ve caused any confusion.

I think the decision Steve asked for is whether to wait until 1.11 before splitting TagsFlagDoc into multiple shorter lines (that flag will indent correctly as of 1.10 but not in 1.9) or do it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.