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

Jump to bottom

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

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

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

mvdan opened this issue Jan 17, 2018 · 9 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@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
@mvdan mvdan added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 17, 2018
@gopherbot gopherbot added this to the Unreleased milestone Jan 17, 2018
@mvdan mvdan mentioned this issue Jan 17, 2018
Closed
@dmitshur
Copy link
Contributor

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
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
Copy link
Contributor

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
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
Copy link
Contributor

spf13 commented Mar 26, 2018

ping @alandonovan for decision

@alandonovan
Copy link
Contributor

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
Copy link
Contributor

dmitshur commented Mar 28, 2018 •
edited

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
Copy link
Contributor

alandonovan commented Mar 28, 2018 via email

@dmitshur
Copy link
Contributor

dmitshur commented Mar 28, 2018 •
edited

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.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@spf13 @dmitshur @mvdan @alandonovan @gopherbot