golang / gddo

Go Doc Dot Org
https://godoc.org
BSD 3-Clause "New" or "Revised" License
1.1k stars 266 forks source link

Do not strip output comments from examples #614

Open nhooyr opened 5 years ago

nhooyr commented 5 years ago

Local godoc doesn't do this. I think its helpful for readers as an assertion for what the test should produce.

It also messes with the formatting. See https://godoc.org/nhooyr.io/websocket#ex-package--Echo

Because the output comment is stripped on the example, the example has an extra line at the end:

    // Now we dial the server and send the messages.
    err = client("ws://" + l.Addr().String())
    if err != nil {
        log.Fatalf("client failed: %v", err)
    }

}
urandom2 commented 5 years ago

godoc simply displays the go/ast.Node contained within go/doc.Example.Code for a given example. gddo attempts to improve on this in doc.printExample by displaying a playable go/ast.File if go/doc.Example.Play != nil.

It appears that something was missed in #238, because while go/doc.Example.Code will contain the // output: comments, go/doc.Example.Play have them stripped by go/doc.stripOutputComment, thus this statement should really be:

} else if n != e.Play {

Furthermore, gddo farms out most all of its ast/doc processing to the standard library, thus the extra new line would be a bug that should be filed against golang/go calling out go/doc.stripOutputComment. Consider that this may be an intentional choice, since there is no requirement that your code block and // output: have any newline separation. In the short term I would suggest removing the new line from your code, but it could be worth the trouble of an issue.

cc: @garyburd and @LaPingvino, since you are the only two who have touched doc.printExample

LaPingvino commented 5 years ago

Why are those stripOutputComment necessary in doc.Example.Play?

urandom2 commented 5 years ago

Why are those stripOutputComment necessary in doc.Example.Play?

I would need to look through the git history to find an explanation of why output is being stripped, but my guess is that the author expected the go/doc.Example.Play code to be executed by a human, thus generating the output, and making the // output: block unnecessary noise/clutter.

Either way, since that is the behaviour of go/doc, I doubt they would change it just for gddo, and go/doc.Example.Output is exposed: it seems reasonable to just set the output box.

LaPingvino commented 5 years ago

There is a bug in godoc atm about porting gddo's behavior for example code (https://github.com/golang/go/issues/31669), which fix by @garyburd deferred to the Play code instead, so it might be a good idea to reevaluate that. If the output: stuff is the reason, it might be better to narrow the kind of comments that is stripped instead in that function and get godoc and gddo on the same page for this...