Provide rule doc links #275
Conversation
Signed-off-by: harupy <hkawamura0130@gmail.com>
| tooltip=f"Jump to {code} documentation", | ||
| target=f"https://docs.astral.sh/ruff/rules/{name}", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Does this iterate over the entire document, checking every line?
| """LSP handler for textDocument/hover request.""" | ||
| document = LSP_SERVER.workspace.get_text_document(params.text_document.uri) | ||
| match = NOQA_REGEX.search(document.lines[params.position.line]) | ||
| def _parse_codes(line: str) -> list[tuple(str, int, int)]: |
There was a problem hiding this comment.
Could we have a NamedTuple / dataclass / TypedDict instead of (str, int, int)? It's hard to know what those represents otherwise.
| document = LSP_SERVER.workspace.get_document(params.text_document.uri) | ||
|
|
||
| links: list[DocumentLink] = [] | ||
| for index, line in enumerate(document.lines): |
There was a problem hiding this comment.
Oh, this request applies for an entire document and not for a specific range. It makes me wonder how often this request is being sent by the client. If it's being sent multiple times then we should think of ways to optimize this as otherwise this might take some time for a large file irrespective of whether it contains the codes or not.
A few options might be:
- Applying the regex on the entire document using
re.finditerinstead of applying the regex on each line - Using
documentLink/resolveto lazily compute the rule name. So, fortextDocument/documentLinkrequest, we would only compute the range and when the client explicitly requests for the link, we would run the command and update thedocumentLinkobject.
There was a problem hiding this comment.
@dhruvmanila Thanks for the comment!
It makes me wonder how often this request is being sent by the client.
- When a file is opened.
- When a file is saved.
There was a problem hiding this comment.
textDocument/documentLink request is also sent on type.
There was a problem hiding this comment.
I'll close this PR :) Iterating over the entire document on type is too expensive.
Summary
Close #274
Test Plan
Manually tested