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

proposal: go/doc: allow custom formatters via an interface #34875

Open
JAicewizard opened this issue Oct 13, 2019 · 14 comments · May be fixed by #37868
Open

proposal: go/doc: allow custom formatters via an interface #34875

JAicewizard opened this issue Oct 13, 2019 · 14 comments · May be fixed by #37868

Comments

@JAicewizard
Copy link

@JAicewizard JAicewizard commented Oct 13, 2019

Recently gopls added markdown output for markdown comments, the implementation is basically a copy of go/doc/comment.go replacing some things with markdown instead of html.
If go/doc was more flexible this would have been less of a copy paste. These should be a interface which can be used to create custom formatters. In this way packages that need backwards compatibility with older go stdlibs can create a formatter and use this with older versions of go. The type implementing this can be copy pasted into go/doc/comment.go(or vice versa).

I am willing to implement this together with porting the gopls markdown implementation to go/doc

@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Oct 15, 2019

/cc @dmitshur

@julieqiu julieqiu added this to the Unreleased milestone Oct 15, 2019
@JAicewizard
Copy link
Author

@JAicewizard JAicewizard commented Nov 27, 2019

Hi, although its been a while I have functional code and an interface that should work in all situations. I have implemented this interface with all the old formats and markdown.
Can somebody check if I should open a CL for it? (not s in check my code, just in general)

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 4, 2019

@JAicewizard: We are currently in the freeze part of the Go release cycle (see https://github.com/golang/go/wiki/Go-Release-Cycle), so no new features will be accepted. I would suggest sending a CL when the tree is opened after Go 1.14 is released.

@JAicewizard
Copy link
Author

@JAicewizard JAicewizard commented Dec 5, 2019

Thanks for the info, I didn't want to start with sending in a CL without any discussion on what the interface should look like, but I will set a reminder to send one in February.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 5, 2019

Sounds good. I think if you have a CL ready, you should be fine to send it in February and discussion can occur during the review process. Otherwise, you can mark this issue as a proposal to spark the conversation (just add the prefix "proposal: " to the issue title).

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 15, 2020

Change https://golang.org/cl/223577 mentions this issue: go/doc: make formatting comments more flexible using an interface

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Mar 18, 2020

Thanks for sending that change, @JAicewizard. It will be helpful to have that prototype when discussing and considering this change.

The go/doc package is low level and in the standard library. Once new API is added to it, it is not possible to change due to the Go 1 compatibility promise. As a result, any potential API additions need to be discussed and agreed on first. (See issue #23864 for an example of a previous proposal to add new functionality to the go/doc package.)

We may decide that adding this new functionality would be better to do in another package (not in the standard library). It depends on whether we can expect the proposed API to solve the problem well today and in the future, without needing further API changes. Until then, it may be less costly to just maintain copies of the code in the necessary packages.

/cc @griesemer @agnivade per owners.

@dmitshur dmitshur modified the milestones: Unreleased, Backlog Mar 18, 2020
@dmitshur
Copy link
Member

@dmitshur dmitshur commented Mar 18, 2020

Also see CL 72890 for related work. It is an attempt to split documentation rendering into smaller and more flexible building blocks. An updated copy of that package is being used to render documentation on pkg.go.dev, which is going to be open sourced (see #36747).

@andybons andybons changed the title go/doc: rework go/doc to be more flexible proposal: go/doc: allow custom formatters via an interface Mar 18, 2020
@gopherbot gopherbot added the Proposal label Mar 18, 2020
@JAicewizard
Copy link
Author

@JAicewizard JAicewizard commented Mar 19, 2020

This implementation is very close to identical to the old implementation for html (to help reduce any added maintenance cost) and allows for very flexible implementations. The implementation is basically given some string and the context around the string to format it in whatever way you would want to.
The idea here is to reduce copying the go standard library, not necessarily to bring something people are already using into the standard library.

I understand that there needs to be discussion first, that is why I opened this issue.

@agnivade
Copy link
Contributor

@agnivade agnivade commented Mar 23, 2020

It's a reasonable proposal. Joe's CL is proof enough that this is needed by libraries out there. The core idea looks good to me. We can hash out the nitty gritties of the new API if the proposal is accepted.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Apr 23, 2020

Per my comment in the CL, I think the interface outlined in the CL is too complicated. We should have a pretty clear idea of what a realistic API looks like before accepting the proposal (not the other way around).

@JAicewizard
Copy link
Author

@JAicewizard JAicewizard commented Apr 23, 2020

Do mean the interface already implemented or the interface outlined in #37868 (comment). My reasoning for the later API is that all comments are essentially a list of text blocks, with headers, urls, idents, and "raw"/code blocks in between.
This interface would give the formatter the ability to do with those blocks of text whatever it wants. This also makes parsing easy, and doesn't put any extra burden on the formatter.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Apr 23, 2020

Sorry for being imprecise. I think the new outlined interface looks great but I haven't spend much time thinking through it.

@JAicewizard
Copy link
Author

@JAicewizard JAicewizard commented Apr 23, 2020

I too think the interface implemented in the CL isn't very good and I didn't actually put much time into thinking about it.

One of the benefits of the new interface that you might not immediately think of is being able to type-assert the formatters compatibility with a potential new feature. You can then adjust the internals to the formatters capabilities. AKA adding a new feature wouldn't have to be breaking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants
You can’t perform that action at this time.