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

x/tools/gopls: support for comment rendering #64936

Open
firelizzard18 opened this issue Jan 2, 2024 · 9 comments
Open

x/tools/gopls: support for comment rendering #64936

firelizzard18 opened this issue Jan 2, 2024 · 9 comments
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@firelizzard18
Copy link
Contributor

firelizzard18 commented Jan 2, 2024

I propose adding a gopls command that uses doc/comment to render documentation comments of package-level declarations. The intent is that this command would be used by a development environment to render pkgsite-style documentation for workspace packages. This could serve the following purposes:

  1. Viewing documentation for code changes that have not been published yet.
  2. Viewing documentation for private repositories.
  3. Reviewing documentation as I write it, to ensure it is formatted the way I want.
  4. Viewing documentation from within my development environment, without needing to switch to a browser.

I'll grant the first three items can be achieved by running pkgsite locally, but that does not address (4), it is less than convenient, and in my experience does not work well for (3).

I propose:

  1. gopls/internal/lsp/cache/docs, which will locate documentation comments, parse them with doc/comment.Parser, and convert the doc/comment.Doc into marshallable types such that the result can be cached.
  2. gopls/internal/lsp/command.Interface.RenderDocs, which will retrieve doc/comment.Docs from a snapshot and render them:
type RenderDocsParams struct {
	Package        protocol.URI
	Declaration    protocol.Location // location of the declaration to generate docs for, optional, omit to render for the entire package
	Format         string            // comment, html, markdown, or text
	HeadingLevel   int               // doc/comment.Printer.HeadingLevel
	HeadingIdTmpl  string            // template for rendering header IDs with doc/comment.Printer.HeadingID
	DocLinkTmpl    string            // template for rendering document links with doc/comment.Printer.DocLinkURL
	DocLinkBaseURL string            // doc/comment.Printer.DocLinkBaseURL
	TextPrefix     string            // doc/comment.Printer.TextPrefix
	TextCodePrefix string            // doc/comment.Printer.TextCodePrefix
	TextWidth      int               // doc/comment.Printer.TextWidth
}

type RenderDocsResult struct {
	Result string
}

CC @hyangah @adonovan

@firelizzard18 firelizzard18 added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Jan 2, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jan 2, 2024
@firelizzard18
Copy link
Contributor Author

@gopherbot, please add label FeatureRequest

@adonovan
Copy link
Member

adonovan commented Jan 2, 2024

(4) Viewing documentation from within my development environment, without needing to switch to a browser.
I'll grant the first three items can be achieved by running pkgsite locally, but that does not address (4), it is less than convenient, and in my experience does not work well for (3).

How would this new protocol-level command be used? Would each LSP client need to add custom logic to invoke it?

If you want to see the complete documentation for a symbol in your editor, jump to its declaration. If you want to see the summary, use your editor's LSP hover feature. If you want to see the complete rendered markup as it would appear on pkg.go.dev, running a local server is the right way to do that (though unfortunately that it is not yet possible, but will be soon). I don't really understand what additional value this command would provide.

@firelizzard18
Copy link
Contributor Author

I don't really understand what additional value this command would provide.

My motivation is that I want an action within VSCode (such as clicking on something) that will bring up an editor view that is basically pkg.go.dev for local packages. I don't want to literally embed pkg.go.dev (or a local version) in a webview - I'd prefer something more seamless. I was thinking a markdown preview. This is something I've wanted for a long time and I keep coming back to. I played around with making my own extension to do this, but I ran into a lot of unpleasantness. I tried using gddo to generate HTML and dump that into a webview, but the code was a mess and the UX was bad. A separate helper process akin to gopls would be better, but I'd basically be recreating a subset of gopls, and a worse version of it. And to make it performant I'd need to reimplement gopls's caching and workspace logic.

How would this new protocol-level command be used? Would each LSP client need to add custom logic to invoke it?

By protocol-level command are you referring to a language server method? I am suggesting a command that would be executed via the workspace/executeCommand LSP method, such as you suggested for #59445, not a new LSP method. Since this is not a standard part of the LSP, yes, each client would need to add custom logic to invoke it. In the case of VSCode, I am imagining some UI element that the user clicks on; then vscode-go executes the command and creates a read-only editor, markdown preview, or webview with the results.

If you want to see the complete documentation for a symbol in your editor, jump to its declaration. [...] If you want to see the complete rendered markup as it would appear on pkg.go.dev, running a local server is the right way to do that (though unfortunately that it is not yet possible, but will be soon).

Personally, I find pkg.go.dev much easier to read than source code when I trying to figure out how to use some package. I've effectively trained myself to go into "How does this code work" whenever I see source code, instead of "How do I call this code" mode, and I find rendered HTML easier to read than comments. Thus I find it much easier to answer "How do I call this code" via pkg.go.dev than via source.

I've tried running pkgsite locally. It works well enough for use cases (1) and (2), but when I run it without any flags it does not work for (3) because once it renders a package, it does not re-render comments if I update them. There's a flag that will change that, but last time I used it things got weird/buggy and I had to restart the process pretty much every time I wanted it to re-render my updates. Regardless/assuming those are bugs and they are fixed, that does not address (4). A big part of my motivation is that I want to see the documentation from within VSCode/without opening a browser, and I want it to feel seamless/fit into VSCode's UX and theming. Hence why I was thinking of using VSCode's markdown preview feature for presentation.

I would be happy to do the work myself, and I'm fine with doing the VSCode/presentation portion in a standalone extension. I opened this issue because doing the work and submitting a CL before discussing it didn't seem like the right move.

@adonovan
Copy link
Member

adonovan commented Jan 2, 2024

I don't want to literally embed pkg.go.dev (or a local version) in a webview - I'd prefer something more seamless.

I see. And I (a stone-age emacs user) thought a webview was the height of integration...

By protocol-level command are you referring to a language server method? I am suggesting a command that would be executed via the workspace/executeCommand LSP method, such as you suggested for #59445, not a new LSP method. Since this is not a standard part of the LSP, yes, each client would need to add custom logic to invoke it.

Got it. From a system design point of view, there's little difference between the two kinds: the client and server both need special logic for each server RPC and for each ad-hoc command.

(I do wish LSP were more composable. For example, I want to implement a new query operation that returns a set of source locations with one-line descriptions, analogous to the output of grep, or a compiler, or a references query, or any other kind of workspace-wide query. It would be nice if there was a way to indicate in the protocol that the result should be displayed using the client's usual UI elements for such things. But instead you get nothing unless you write logic for n different clients...)

Personally, I find pkg.go.dev much easier to read than source code when I trying to figure out how to use some package. I've effectively trained myself to go into "How does this code work" whenever I see source code, instead of "How do I call this code" mode, and I find rendered HTML easier to read than comments. Thus I find it much easier to answer "How do I call this code" via pkg.go.dev than via source.

Yeah, I agree. The API docs provide a clearer overview, and keep you honest about interface vs implementation. They're also useful for previewing how you new code would appear to its users.

I've tried running pkgsite locally. It works well enough for use cases (1) and (2), but when I run it without any flags it does not work for (3) because once it renders a package, it does not re-render comments if I update them. There's a flag that will change that, but last time I used it things got weird/buggy and I had to restart the process pretty much every time I wanted it to re-render my updates. Regardless/assuming those are bugs and they are fixed,

Yep, we will fix all those things this year.

...that does not address (4). A big part of my motivation is that I want to see the documentation from within VSCode/without opening a browser, and I want it to feel seamless/fit into VSCode's UX and theming. Hence why I was thinking of using VSCode's markdown preview feature for presentation.

I see. It still seems to me that the marginal value over a VS Code webview will be rather small, not enough to justify adding support for a new protocol-level request to N editors. It also seems duplicative: rendering Go docs beautifully is pkgsite's job, and this feature would require gopls to know how to do it as well (and lead to a stream of issues when the two tools differ).

I wonder whether this might be something that can be squeezed into the existing protocol, perhaps as the hover response for a "package foo" declaration, which doesn't currently appear to generate any hover text---though I can imagine the complete package doc is too much for a hovering popup.

I would be happy to do the work myself, and I'm fine with doing the VSCode/presentation portion in a standalone extension. I opened this issue because doing the work and submitting a CL before discussing it didn't seem like the right move.

Thanks for the offer, and for discussing this first.

@firelizzard18
Copy link
Contributor Author

I do wish LSP were more composable.

There have been some discussions related to this. The LSP spec includes a Base protocol specification, which seems like an answer to your desire to a degree. Though extensible != composable, but it's something worth being aware of.

It still seems to me that the marginal value over a VS Code webview will be rather small,

I think this, along with within-vscode vs browser and formatting, is subjective, so the most I can do is explain my own perspective. Having something within VSCode is worth the time I'd invest in this since every time I alt-tab to another window, it disrupts my workflow to a degree (especially if I'm switching between more than two windows). My dissatisfaction with using a webview is similar - embedding a website directly feels clunky, because the UI and styling is quite different, there are UI components that are just extra noise in this context (mainly navigation), links can be tricky, etc. I have tried to resolve those issues by injecting styles and/or scripts and/or modifying the HTML, but that's a mess. Ultimately it means that it's much harder and potentially infeasible to make it feel seamless, and thus more mental overhead to compensate for that dissonance.

With respect to formatting, that's even more of a subjective value judgement. Personally I believe I would be satisfied with some markdown thrown at VSCode's built-in renderer. I do not expect my development environment to be pkgsite-level beautiful. For example, being able to do something like this is far more important to me:

  1. I select a method and execute "Open documentation" (via right click, command pallet, etc).
  2. That opens the documentation for the package the method belongs to and scrolls to the method.
  3. I find some other declaration I want to jump to; e.g. clicking on the return type, or a "see also" link in the method's documentation, or scrolling through the package docs until I find what I want.
  4. I click on the new declaration to jump (within vscode) to its source code, or I right click on it and execute "Find all references".

My point is, nicely formatted documentation is low on my priority list; more important to me are minimizing context switching, distractions, and other dissonance (such as switching between dark-mode code and light-mode pkgsite) and being able to easily jump around, from code to docs, docs to docs, and docs to code (also code to code, but that's already covered).

...not enough to justify adding support for a new protocol-level request to N editors

My immediate response, with my "I'm a user" hat on, is that this is a convenience feature the language server is providing and thus not something that needs to be implemented for every editor. But when I think about it more and put my maintainer hat on, I can see that implementing it for one editor would likely lead to users asking, "Why doesn't <my editor> have this feature" and thus pressure to implement it for all editors.

It also seems duplicative: rendering Go docs beautifully is pkgsite's job, and this feature would require gopls to know how to do it as well (and lead to a stream of issues when the two tools differ).

I'm not proposing adding any rendering code; I want to reuse doc/comment's rendering:

type RenderDocsParams struct {
	Format string // comment, html, markdown, or text
	// ...
}

type RenderDocsResult struct {
	Result []string
}

func RenderDocs(ctx context.Context, params RenderDocsParams) (RenderDocsResult, error) {
	var result RenderDocsResult
	docs, err := retrieveDocs(ctx, params)
	if err != nil {
		return result, err
	}

	printer := configureCommentPrinter(params)
	var render func(*comment.Doc) []byte
	switch params.Format {
	case "comment":
		render = printer.Comment(doc)
	case "html":
		render = printer.HTML(doc)
	case "markdown":
		render = printer.Markdown(doc)
	case "text":
		render = printer.Text(doc)
	default:
		return result, fmt.Errorf("unsupported format %q", params.Format)
	}

	for _, doc := range docs {
		result.Result = append(result.Result, string(render(doc)))
	}
	return result, nil
}

On the vscode-go side, I'm suggesting asking gopls for markdown and letting VSCode handle rendering. I wrote a small essay above about formatting so I won't reiterate here.

@hyangah
Copy link
Contributor

hyangah commented Jan 3, 2024

I'm suggesting asking gopls for markdown and letting VSCode handle rendering.

This is aligned with what I've been expecting. I agree pkgsite UI isn't a good fit for in-editor viewer. Its use of javascript and pkgsite service oriented design doesn't blend well. Heavy-weight webview is not a recommended best practice in VS Code either.

A custom command (e.g. gopls.render_package) that takes doc URI and optional parameters (e.g. include unexported symbols, include indexes, ...) and returns bytes will be helpful. This also doesn't require to start a web server with port, which is an added benefit. Not sure about all Format other than markdown or HeadingLevel, HeadingIdTmpl, ... yet but we can expand the request message type as we find more use cases.

I experimented with princjef/gomarkdoc with some modification + vscode markdown renderer and got a pretty promising output. It makes sense to offer this info from gopls (unless go doc expands its functionality to produce html/markdown format full package doc.).

@hyangah
Copy link
Contributor

hyangah commented Jan 3, 2024

Update: it seems that vscode's default markdown preview's functionality is quite limited. For example, we will probably want some links to point to files outside the workspace (e.g. file in module cache or stdlib), but the vscode's default markdown preview does not support file:/// uri (microsoft/vscode#182870, ...), nor true relative/absolute paths ( microsoft/vscode#120754, microsoft/vscode#139289, ...). MS VSCode team seems to recommend markdown extensions or custom solution. :-( I think a simple, basic html (without any fancy styling or javascript) is better in this case.

@firelizzard18
Copy link
Contributor Author

Not sure about all Format other than markdown or HeadingLevel, HeadingIdTmpl, ... yet but we can expand the request message type as we find more use cases.

I was thinking along the lines of exposing most/all of the capabilities of doc/comment.Printer against unknown future needs/clients other than VSCode, but I'm fine with starting small. Though I think allowing customization of doc links is a good idea, since we probably want those to execute VSCode commands.

I experimented with princjef/gomarkdoc

Does that have advantages over go/doc/comment.Printer.Markdown? It looks like it has its own custom templates. My inclination is to use the standard library's markdown generator unless there's a compelling reason to use a third party library.

It seems that vscode's default markdown preview's functionality is quite limited.

That is rather unfortunate 😞. My inclination is to initially live with the limitations of throwing markdown at vscode, and then later work add a custom markdown renderer to vscode-go with more bells and whistles.

@hyangah
Copy link
Contributor

hyangah commented Jan 3, 2024

Does that have advantages over go/doc/comment.Printer.Markdown?

go/doc/comment.Printer.Markdown itself isn't sufficient to generate a markdown version of package doc.
(princjef/gomarkdoc, go doc, pkgsite, and gopls hover formatting all use go/doc/comment underneath to convert selected comment blocks, but wrap the markdown printer output pieces in templates formulated for their needs). I just used princjef/gomarkdoc just to quickly test markdown output against vscode's markdown previewer and demonstrate why we need better integration than just wrapping pkgsite in webview.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants