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

cmd/doc: odd formatting when last method lacks documentation #30492

Closed
dsnet opened this issue Mar 1, 2019 · 4 comments
Closed

cmd/doc: odd formatting when last method lacks documentation #30492

dsnet opened this issue Mar 1, 2019 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.13

Comments

@dsnet
Copy link
Member

dsnet commented Mar 1, 2019 •
edited

Using go1.12

$ go doc -all io | grep -C 8 "LimitedReader) Read"
type LimitedReader struct {
	R Reader // underlying reader
	N int64  // max bytes remaining
}
    A LimitedReader reads from R but limits the amount of data returned to just
    N bytes. Each call to Read updates N to reflect the new amount remaining.
    Read returns EOF when N <= 0 or when the underlying R returns EOF.

func (l *LimitedReader) Read(p []byte) (n int, err error)
type PipeReader struct {
	// Has unexported fields.
}
    A PipeReader is the read half of a pipe.

func (r *PipeReader) Close() error
    Close closes the reader; subsequent writes to the write half of the pipe
    will return the error ErrClosedPipe.

The LimitedReader.Read method has no documentation, so does not print an empty line below it. This makes the output odd as it visually looks like LimitedReader.Read is more related to PipeReader than it does with LimitedReader.

What I expected to see:

 func (l *LimitedReader) Read(p []byte) (n int, err error)
+
 type PipeReader struct {
 	// Has unexported fields.
 }
@dsnet dsnet changed the title cmd/doc: odd formatting when methods lack documentation cmd/doc: odd formatting when last method lacks documentation Mar 1, 2019
@agnivade
Copy link
Contributor

agnivade commented Mar 1, 2019

Interesting. On a different note, the Read method should be documented :)

/cc @robpike

@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 1, 2019
@agnivade agnivade added this to the Go1.13 milestone Mar 1, 2019
@agnivade
Copy link
Contributor

agnivade commented Mar 1, 2019

Found it. I have 2 possible fixes for it.

One is unconditionally change 1 to 2 here in func (pkg *Package) emit.

} else {
	pkg.newlines(1)
}

Interestingly this was done to fix #12756 filed by @dsnet himself, which was also about the func (l *LimitedReader) Read method.

There are side-effects to this approach. Since we are always printing a gap after a method, so now functions inside interfaces without a comment do not have a gap.

Before --

$gotip run .  io.read
func (l *LimitedReader) Read(p []byte) (n int, err error)
func (r *PipeReader) Read(data []byte) (n int, err error)
    Read implements the standard Read interface: it reads data from the pipe,
    blocking until a writer arrives or the write end is closed. If the write end
    is closed with an error, that error is returned as err; otherwise err is
    EOF.

func Read(p []byte) (n int, err error)
func (s *SectionReader) Read(p []byte) (n int, err error)

After--

$gotip run .  io.read
func (l *LimitedReader) Read(p []byte) (n int, err error)

func (r *PipeReader) Read(data []byte) (n int, err error)
    Read implements the standard Read interface: it reads data from the pipe,
    blocking until a writer arrives or the write end is closed. If the write end
    is closed with an error, that error is returned as err; otherwise err is
    EOF.

func Read(p []byte) (n int, err error)
func (s *SectionReader) Read(p []byte) (n int, err error)

To make it consistent, we would need to add a gap after functions too.

Another alternative is cleaner, but conditional. We check if a fun.Doc is empty or not in func (pkg *Package) typeDoc and accordingly add a gap. This is clinical and does not affect anything else.

@robpike for decision.

@agnivade agnivade added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 1, 2019
@robpike
Copy link
Contributor

robpike commented Mar 2, 2019

I'd go for checking fun.Doc but it doesn't really matter much. Either solution is fine.

@agnivade agnivade self-assigned this Mar 2, 2019
@agnivade agnivade added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Mar 2, 2019
@gopherbot
Copy link

Change https://golang.org/cl/166178 mentions this issue: cmd/doc: add a line gap after a method with no comment

@golang golang locked and limited conversation to collaborators Mar 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
4 participants
@agnivade @robpike @dsnet @gopherbot