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

Ability to add links to diagnostics and render them as links in problems view #11847

Closed
darkred opened this issue Sep 11, 2016 · 47 comments
Closed
Assignees
Labels
api api-finalization error-list Problems view feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@darkred
Copy link

darkred commented Sep 11, 2016

For example, linting var test = 1
results in these warnings in the Problems panel:
2016-09-11_185535
My suggestion:
the no-unused-vars and semi to be clickable and to link to
http://eslint.org/docs/rules/no-unused-vars and to
http://eslint.org/docs/rules/semi
respectively.

(I've found this feature in AtomLinter and it's really useful)

cc: @dbaeumer


PS. Using vscode-eslint 1.10.18 with ESLint 3.5.0 in VSCode 1.5.1 using win10 x64.
@dbaeumer
Copy link
Member

Like the idea of having a link in the problems panel to point to documentation. @sandy081

@sandy081
Copy link
Member

@dbaeumer Does the problem diganostics have information about these external links?

@dbaeumer
Copy link
Member

@sandy081 no we would need to add it.

@sandy081 sandy081 added error-list Problems view feature-request Request for new features or functionality labels Sep 13, 2016
@sandy081 sandy081 added this to the Backlog milestone Sep 13, 2016
@bpugh
Copy link

bpugh commented Feb 4, 2017

I would like to take a stab at this if no work has already been done. FYI looks the Atom linter-eslint folks pulled this logic out into a standalone module we could use pretty easily: https://github.com/jfmengels/eslint-rule-documentation

@sandy081
Copy link
Member

sandy081 commented Feb 6, 2017

@bpugh That would be great. No work has not been started. Let me know if you need any assistance.

@bpugh
Copy link

bpugh commented Feb 6, 2017

Thanks! I did have one question though. It looks pretty straightforward to add the url to the message from vscode-eslint but we're currently using the diagnostics api to display the messages in the problems panel but it doesn't seem to allow anyway to link to an external url. Would this require a PR to vscode as well?

@rmartins90
Copy link

+1

@gerrywastaken
Copy link

gerrywastaken commented Jul 22, 2017

@sandy081 I believe @bpugh was asking you that question but didn't tag you.

p.s. @rmartins90 @kuschti You can thumbs up a comment directly instead of adding spam.

@sandy081 sandy081 changed the title [vscode-eslint] Suggestion: links to the documentation for each rule in the Problems panel Ability to add links to diagnostics and render them as links in problems view Jul 24, 2017
@sandy081
Copy link
Member

@bpugh Yes, this needs to be implemented in VS Code too, to detect and render links in Problems view.

@selrond
Copy link
Contributor

selrond commented Oct 20, 2017

No progress on this one?
Would be totally handy!

@jeffwilcox
Copy link

I'd be a fan, too, including linking to optional external URLs... for example, I have an extension that integrates with known vulnerabilities for certain components; I'd love to be able to link in the problem space to the actual advisory URL with more details... today folks have to copy/paste from the Diagnostic message/text to get the URL out.

@callagga
Copy link

+1

@LitileXueZha
Copy link

Hey guys, how are things going

@thibaudcolas
Copy link

thibaudcolas commented Oct 15, 2018

I would like to work on this if no one else is. I stumbled upon this as part of Microsoft's Hacktoberfest participation! This is my first time working in the vscode codebase so I spent a bit of time looking into how to achieve this. Here is my progress so far.

Reference implementation – Atom and its linter-eslint

Uses ESLint's rules metadata, or eslint-rule-documentation if no metadata is available https://github.com/AtomLinter/linter-eslint/blob/4b7334005b58f4dfbcdd239a2eed6e516d8ffe4f/src/rules.js#L39-L56

The rules metadata is only exposed starting with ESLint v4. Ideally would be retrieved with CLIEngine#getRules(), introduced in v4.15.0. So eslint-rule-documentation is a fallback for older versions, and third-party plugins that do not expose rules metadata (yet).

See https://github.com/AtomLinter/linter-eslint/blob/3630ab63e853bb98afd99e142143642b1569708b/src/worker-helpers.js#L189-L211 for the rules retrieval from ESLint.

This is what it looks like in the end:

atom-linter-eslint

microsoft/vscode-eslint#151

VS Code implementation

vscode part

Introduce a new url attribute on the Diagnostic API, then different parts of the UI can choose whether to use this URL to render a link, or not.

  • Some parts of the UI already support rendering links via Markdown, so it shouldn't be too much work.
  • I'm still struggling to figure out which widget to do this inside of 😅.

WIP: https://github.com/Microsoft/vscode/compare/master...thibaudcolas:feature/diagnostics-links-11847?expand=1

vscode-eslint part (or any other extension)

Use the new url attribute of the Diagnostic, populating it based on the metadata from CLIEngine#getRules() if available, or eslint-rule-documentation otherwise.

Add an optional getRules here: https://github.com/Microsoft/vscode-eslint/blob/ddc2dff0b5d01045b635cd55975a1e01fef8d732/server/src/eslintServer.ts#L147-L149

Load (and cache?) the rules from ESLint (with getRules) somewhere – I'm not too familiar with the extensions lifecycle just yet.

Set the URL in makeDiagnostic: https://github.com/Microsoft/vscode-eslint/blob/ddc2dff0b5d01045b635cd55975a1e01fef8d732/server/src/eslintServer.ts#L160-L178

WIP: https://github.com/Microsoft/vscode-eslint/compare/master...thibaudcolas:feature/rule-docs-151?expand=1


Next step is for me to figure out where exactly the Diagnostics are displayed, and then render a Markdown link there if the url is present. Perhaps also add a "code action" available via the contextual menu to open this URL in the browser.

vscode-rules-link

@thibaudcolas
Copy link

Progress update – I think I figured out all of the places that would/could support those links:

diagnostic-link-ui

  • The hover modal, where at the moment content is displayed as a code block (I added the link shown in the screenshot)
  • The F8 panel
  • The Problems tree viewer, with its highlighted labels for filtering

If anyone has thoughts on how to handle the URLs in each of these, please let me know.

@jrieken
Copy link
Member

jrieken commented Feb 5, 2020

wrt to the API proposal, we should use target (or uri) instead of link. The former would align this with the DocumentLink-api.

@henryju
Copy link

henryju commented Feb 10, 2020

In SonarLint we are already allowing users to open rule description (using a code action), so this feature would be useful as a replacement. But we are displaying rule description directly inside VSCode (using a web view panel). Do you know if this feature would support URI with custom scheme, so that we could use our own handler and not open an external browser?

@jrieken
Copy link
Member

jrieken commented Feb 10, 2020

Yeah, using whatever uri-scheme should be supported by this but I do wonder why SonarLint uses a web view and not just a browser? Is that your help text is dynamic or private?

@henryju
Copy link

henryju commented Feb 10, 2020

Our help text is indeed dynamically generated from metadata coming from the embedded analyzer (rule name, severity, markdown description converted to HTML, ...). This is better for us to rely on embedded analyzer metadata than trying to host online content, since it will perfectly match the analyzer version, and also this will work when user has no internet connexion.

@jrieken
Copy link
Member

jrieken commented Feb 10, 2020

Ok, that makes sense. For us it is important that the link has a direct connection to the error-code, hence the shape of the API and that it isn't a generic link

octref added a commit that referenced this issue Feb 10, 2020
@jvilk-stripe
Copy link

👋 would this be added to the language server protocol? This is a very useful feature for surfacing documentation for errors.

@dbaeumer
Copy link
Member

Yes, this will be added to the LSP as well.

@octref
Copy link
Contributor

octref commented Feb 27, 2020

Steps:

  • Read this API:

    vscode/src/vs/vscode.d.ts

    Lines 4676 to 4691 in bddc5ef

    /**
    * A code or identifier for this diagnostic.
    * Should be used for later processing, e.g. when providing [code actions](#CodeActionContext).
    */
    code?: string | number | {
    /**
    * A code or identifier for this diagnostic.
    * Should be used for later processing, e.g. when providing [code actions](#CodeActionContext).
    */
    value: string | number;
    /**
    * A target URI to open with more information about the diagnostic error.
    */
    target: Uri;
    };
  • Write an extension that uses this API. Make sure if you provide target, a link is rendered with the value as text and upon clicking opens the link in problems-view/inline-error-view/error-hover. Here's how it works roughly: Ability to add links to diagnostics and render them as links in problems view #11847 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization error-list Problems view feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.