Go: text/template/parse: add CommentNode to the parse tree

[a8m]

Currently, template comments are ignored in the lexer and do not show in the parse tree.
It makes it difficult to build tools around templates (like code formatter) without re-implementing the parser.

My proposal is to add a CommentNode to the parse tree and skip it in the executer.

[mvdan]

I assume this can only be accepted if https://github.com/golang/go/issues/25357 is rejected.

[a8m]

I assume this can only be accepted if #25357 is rejected.

Why? It's a Go2 proposal, and text/template/parse is a public package that's being used outside.

[mvdan]

Yes; but if the plan is to eventually hide/remove the template/parse package, considering API changes to it wouldn't make a lot of sense. At the very least, the two proposals are related.

[rsc]

/cc @robpike for thoughts about supporting template-manipulation programs a la gofmt by adding CommentNode.

[a8m]

I think it’s needed for gopls as well - #36911

[robpike]

I would have made the package internal had that been an option at the time, but I couldn't, and now it's being used by software that wouldn't have been able to, making it harder to maintain and de facto subject to the compatibility rules. From my position, that is a tale of woe. Hyrum's law, which was a problem before Hyrum was even born, bites again.

On the other hand, people do depend on it and the uses are mostly reasonable, so we should probably accept this and close #25357. I do think though that we should keep the comment that discourages its use, to give us some leverage against unreasonable requests for change.

[rsc]

Based on the discussion above, then, this sounds like a likely accept.
For what it's worth, I might have made regexp/syntax an internal package too, but it has proved so useful in so many other contexts that I'm glad I exposed it.

[mvdan]

Sounds good to me. Should we reject https://github.com/golang/go/issues/25357, then? In the past few years it's been made clear that Go2 will be an incremental process instead of one big set of breaking changes, so it seems unlikely to me that we'd hide an entire package at any point.

[a8m]

Thanks for the feedback. I would be happy to add this once it's accepted.

Should we reject #25357, then?

IMHO, the template package should provide an API for tools like gopls to interact with it. Like the go package for Go.
Otherwise, what's the alternative, reimplementing it externally? This is what I do currently, but I expect to replace it once gopls will add support for it (#36911).

[rsc]

No change in consensus, so accepted. Yes, I think we can reject #25357 as well.
For better or worse, this package is the equivalent of go/ast for templates.
In a different timeline we might have made it an internal-only package and
prepared an API that was specifically meant for public consumption, but that's
not what happened.

[carlmjohnson]

If this has been accepted, then I take it there's no interest in marking text/template/parse as deprecated?

[a8m]

If this has been accepted, then I take it

Since I already have a working change for this in my fork that I plan to push in the upcoming days, I prefer to take it. Thanks

[gopherbot]

Change https://golang.org/cl/229398 mentions this issue: text/template: add CommentNode to template parse tree

[ianlancetaylor]

The current CL is adding a new ParseWithOptions function; see #38627.

[a8m]

Hey, CL229398 is currently blocked on review. There's another discussion on #38627 about making the API more generic for changes in the future.

Any chance to get help with the review? I really want to see it shipped with 1.15.