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

Hover documentation is truncated for some symbols #65

Closed
hauserx opened this issue Dec 4, 2024 · 1 comment · Fixed by #66
Closed

Hover documentation is truncated for some symbols #65

hauserx opened this issue Dec 4, 2024 · 1 comment · Fixed by #66

Comments

@hauserx
Copy link
Contributor

hauserx commented Dec 4, 2024

See most prominent example here:

image

Looks like vscode is accepting some tags, but for uknown it simply removes content.

Possible solution 1: change vscode bazel extension

Applying this patch to vscode-bazel seems to do the trick:

--- a/src/extension/extension.ts
+++ b/src/extension/extension.ts
@@ -159,6 +159,7 @@ function createLsp(config: vscode.WorkspaceConfiguration) {

   const clientOptions: LanguageClientOptions = {
     documentSelector: [{ scheme: "file", language: "starlark" }],
+    markdown: { supportHtml: true },
   };

image

image

Possible solution 2: Sanitize documentation

Remove html tags or/and replace those with marksdown
Possible crate to use, seems pretty lightweight: https://github.com/letmutex/htmd

Additional example: nvim

Example does not seem pretty in nvim as well (at least with my config). Not sure whether can teach nvim to understand html, possibly then option 2 would be better.

image

@hauserx
Copy link
Contributor Author

hauserx commented Dec 5, 2024

Couple of points noted when creating a solution for the immediate issue:

  • It would be really nice if bazel exposed the built in symbols with documentation through bazel info, building it using gendoc in bazel source seems sub-optimal
  • The current documentation harvested by gen_api_proto is tuned to create https://bazel.build/rules/lib/overview :
  • The gen_api_proto does not expose information about WORKSPACE, MODULE and other symbols, but with small tweaks it can be changed to do. I will try to send a PR to bazel (Bazel exports methods for other types, but those are not used by ApiExporter through libraries it calls).
  • Maybe buildin binary file could be replaced with text file (like pbtxt or json), just to avoid having binary files in the repo.
  • builtins.bin contain types and functions with the same names. Right now documentation from functions is presented in LSP, but types have better documentation (e.g. DefaultInfo function has The DefaultInfo constructor while the DefaultInfo type has much more extensive documentation)

hauserx added a commit to hauserx/bazel-lsp that referenced this issue Dec 5, 2024
Documentation for builtin starlark methods contains HTML tags, while LSP
clients handle markdown much easier. Used htmd crate to convert HTML
tags to markdown, and additionally change links from relative to
pointing to bazel website.

Fixes cameron-martin#65
cameron-martin pushed a commit that referenced this issue Dec 9, 2024
The binary .pb files are now generated during build and embedded as
previously in the binary itself. But repo contains text proto versions
of the files.

This provides more transparency about what is built into the binary
itself, and more visibility into what PRs change around those files (for
more informed reviews as well as less security concerns).

Addresses one of the points mentioned in
#65 (comment)
hauserx added a commit to hauserx/bazel-lsp that referenced this issue Dec 10, 2024
Documentation for builtin starlark methods contains HTML tags, while LSP
clients handle markdown much easier. Used htmd crate to convert HTML
tags to markdown, and additionally change links from relative to
pointing to bazel website.

Fixes cameron-martin#65
cameron-martin pushed a commit that referenced this issue Dec 12, 2024
Documentation for builtin starlark methods contains HTML tags, while LSP
clients handle markdown much easier. Used htmd crate to convert HTML
tags to markdown, and additionally change links from relative to
pointing to bazel website.

Fixes #65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant