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

"Goto Diagnostic" quick panels. #1884

Merged
merged 16 commits into from
Nov 6, 2021
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Default.sublime-commands
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@
"caption": "LSP: Goto Declaration",
"command": "lsp_symbol_declaration",
},
{
"caption": "LSP: Goto Diagnostic",
rchl marked this conversation as resolved.
Show resolved Hide resolved
"command": "lsp_goto_diagnostic",
"args": {"uri": "$view_uri"}
},
{
"caption": "LSP: Goto Diagnostic in Project",
"command": "lsp_goto_diagnostic"
},
{
"caption": "LSP: Toggle Log Panel",
"command": "lsp_toggle_server_panel",
Expand Down
27 changes: 27 additions & 0 deletions Default.sublime-keymap
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,33 @@
// }
// ]
// },
// Goto Diagnostic
// {
// "command": "lsp_goto_diagnostic",
// "args": {
// "uri": "$view_uri"
// },
// "keys": [
// "f8"
// ],
// "context": [
// {
// "key": "setting.lsp_active"
// }
// ]
// },
// Goto Diagnostic in Project
// {
// "command": "lsp_goto_diagnostic",
// "keys": [
// "shift+f8"
// ],
// "context": [
// {
// "key": "setting.lsp_active"
// }
// ]
Comment on lines +276 to +280
Copy link
Member

@rchl rchl Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that this one should have no context check because it should still work even if current file doesn't have any session.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is supposed to work for all files in project, even for those without a session, and it does.

Copy link
Member

@rchl rchl Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does work indeed but this context check is wrong because, if it would work, it would only allow this command to run when the active file has active session.

The reason it doesn't cause problems is due to this being a WindowCommand so our custom on_query_context handler is not even queried and the check always passes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or more precisely, the setting.lsp_active key is internally matched against the view's settings object but since this is not a TextCommand, it doesn't do what it looks like.

// },
// Rename
// {
// "command": "lsp_symbol_rename",
Expand Down
9 changes: 9 additions & 0 deletions Main.sublime-menu
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,15 @@
"caption": "LSP: Goto Implementation…",
"command": "lsp_symbol_implementation"
},
{
"caption": "LSP: Goto Diagnostic…",
"command": "lsp_goto_diagnostic",
"args": {"uri": "$view_uri"}
},
{
"caption": "LSP: Goto Diagnostic in Project…",
"command": "lsp_goto_diagnostic"
},
{
"caption": "LSP: Find References",
"command": "lsp_symbol_references"
Expand Down
1 change: 1 addition & 0 deletions boot.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
from .plugin.goto import LspSymbolDefinitionCommand
from .plugin.goto import LspSymbolImplementationCommand
from .plugin.goto import LspSymbolTypeDefinitionCommand
from .plugin.goto_diagnostic import LspGotoDiagnosticCommand
from .plugin.hover import LspHoverCommand
from .plugin.panels import LspShowDiagnosticsPanelCommand
from .plugin.panels import LspToggleServerPanelCommand
Expand Down
3 changes: 2 additions & 1 deletion dependencies.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
{
"*": {
">=3124": [
">=4096": [
"backrefs",
"bracex",
"mdpopups",
"pathlib",
"pyyaml",
"wcmatch"
]
Expand Down
106 changes: 74 additions & 32 deletions plugin/core/diagnostics_manager.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from .protocol import Diagnostic, DiagnosticSeverity
from .settings import userprefs
from .typing import Callable, Iterator, List, Optional, Tuple
from .protocol import Diagnostic, DiagnosticSeverity, DocumentUri
from .typing import Callable, Iterator, List, Tuple, TypeVar
from .url import parse_uri
from .views import diagnostic_severity, format_diagnostic_for_panel
from .views import diagnostic_severity
from collections import OrderedDict
import functools

ParsedUri = Tuple[str, str]
T = TypeVar('T')


class DiagnosticsManager(OrderedDict):
Expand All @@ -17,44 +20,83 @@ class DiagnosticsManager(OrderedDict):
#
# https://microsoft.github.io/language-server-protocol/specification#textDocument_publishDiagnostics

def add_diagnostics_async(self, uri: str, diagnostics: List[Diagnostic]) -> None:
_, path = parse_uri(uri)
def add_diagnostics_async(self, document_uri: DocumentUri, diagnostics: List[Diagnostic]) -> None:
"""
Add `diagnostics` for `document_uri` to the store, replacing previously received `diagnoscis`
for this `document_uri`. If `diagnostics` is the empty list, `document_uri` is removed from
the store. The item received is moved to the end of the store.
"""
uri = parse_uri(document_uri)
if not diagnostics:
# received "clear diagnostics" message for this path
self.pop(path, None)
# received "clear diagnostics" message for this uri
self.pop(uri, None)
return
max_severity = userprefs().diagnostics_panel_include_severity_level
self[path] = (
list(
filter(
None,
(
format_diagnostic_for_panel(diagnostic)
for diagnostic in diagnostics
if diagnostic_severity(diagnostic) <= max_severity
),
)
),
len(list(filter(has_severity(DiagnosticSeverity.Error), diagnostics))),
len(list(filter(has_severity(DiagnosticSeverity.Warning), diagnostics))),
)
self.move_to_end(path) # maintain incoming order
self[uri] = diagnostics
self.move_to_end(uri) # maintain incoming order

def filter_map_diagnostics_async(self, pred: Callable[[Diagnostic], bool],
f: Callable[[ParsedUri, Diagnostic], T]) -> Iterator[Tuple[ParsedUri, List[T]]]:
"""
Yields `(uri, results)` items with `results` being a list of `f(diagnostic)` for each
diagnostic for this `uri` with `pred(diagnostic) == True`, filtered by `bool(f(diagnostic))`.
Only `uri`s with non-empty `results` are returned. Each `uri` is guaranteed to be yielded
not more than once. Items and results are ordered as they came in from the server.
"""
for uri, diagnostics in self.items():
results = list(filter(None, map(functools.partial(f, uri), filter(pred, diagnostics)))) # type: List[T]
htmue marked this conversation as resolved.
Show resolved Hide resolved
if results:
yield uri, results

def diagnostics_panel_contributions_async(
self,
) -> Iterator[Tuple[str, List[Tuple[str, Optional[int], Optional[str], Optional[str]]]]]:
for path, (contribution, _, _) in self.items():
if contribution:
yield path, contribution
def filter_map_diagnostics_flat_async(self, pred: Callable[[Diagnostic], bool],
f: Callable[[ParsedUri, Diagnostic], T]) -> Iterator[Tuple[ParsedUri, T]]:
"""
Flattened variant of `filter_map_diagnostics_async()`. Yields `(uri, result)` items for each
of the `result`s per `uri` instead. Each `uri` can be yielded more than once. Items are
grouped by `uri` and each `uri` group is guaranteed to appear not more than once. Items are
ordered as they came in from the server.
"""
for uri, results in self.filter_map_diagnostics_async(pred, f):
for result in results:
yield uri, result

def sum_total_errors_and_warnings_async(self) -> Tuple[int, int]:
"""
Returns `(total_errors, total_warnings)` count of all diagnostics currently in store.
"""
return (
sum(errors for _, errors, _ in self.values()),
sum(warnings for _, _, warnings in self.values()),
sum(map(severity_count(DiagnosticSeverity.Error), self.values())),
sum(map(severity_count(DiagnosticSeverity.Warning), self.values())),
)

def diagnostics_by_document_uri(self, document_uri: DocumentUri) -> List[Diagnostic]:
"""
Returns possibly empty list of diagnostic for `document_uri`.
"""
return self.get(parse_uri(document_uri), [])

def diagnostics_by_parsed_uri(self, uri: ParsedUri) -> List[Diagnostic]:
"""
Returns possibly empty list of diagnostic for `uri`.
"""
return self.get(uri, [])


def severity_count(severity: int) -> Callable[[List[Diagnostic]], int]:
def severity_count(diagnostics: List[Diagnostic]) -> int:
return len(list(filter(has_severity(severity), diagnostics)))

return severity_count
Comment on lines +84 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be a lambda?

Suggested change
def severity_count(severity: int) -> Callable[[List[Diagnostic]], int]:
def severity_count(diagnostics: List[Diagnostic]) -> int:
return len(list(filter(has_severity(severity), diagnostics)))
return severity_count
def severity_count(severity: int) -> Callable[[List[Diagnostic]], int]:
return lambda diagnostics: len(list(filter(has_severity(severity), diagnostics)))



def has_severity(severity: int) -> Callable[[Diagnostic], bool]:
def has_severity(diagnostic: Diagnostic) -> bool:
return diagnostic_severity(diagnostic) == severity

return has_severity
Comment on lines 91 to 95
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe turn this into a lambda as well?

Suggested change
def has_severity(severity: int) -> Callable[[Diagnostic], bool]:
def has_severity(diagnostic: Diagnostic) -> bool:
return diagnostic_severity(diagnostic) == severity
return has_severity
def has_severity(severity: int) -> Callable[[Diagnostic], bool]:
return lambda diagnostic: diagnostic_severity(diagnostic) == severity

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These I would leave as they are because it's immediately clear from the layout that they are just curried functions, with lambdas this would be a little harder to get.

A proper @curry decorator would be nice, though:

@curry
def is_severity_included(max_severity: ..., diagnostic: ...) -> ...:
    return diagnostic_severity(diagnostic) <= max_severity

There is one in the toolz package, but it seems overkill here and I don't know how well it plays with typing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally not accustomed to the use of curried functions. I see a callable as return type and would expect a lambda if it's a single line. I think this is true for a lot of Python (or imperative) developers. But, I guess your argumentation makes sense as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One downside of lambdas is that those can't be type-annotated properly.

Copy link

@FichteFoll FichteFoll Nov 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend functools.partial, but iirc mypy also had problems with that. No idea about pyright.

Currying is really cool, but it's not native to Python and will forever feel alien unless it becomes integrated and more Python developers get to know it. The fact that a calling any function may return another function if it isn't provided with all its requested arguments just doesn't apply to languages without this concept at its core.



def is_severity_included(max_severity: int) -> Callable[[Diagnostic], bool]:
def severity_included(diagnostic: Diagnostic) -> bool:
return diagnostic_severity(diagnostic) <= max_severity

return severity_included
Comment on lines +98 to +102
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lambda?

Suggested change
def is_severity_included(max_severity: int) -> Callable[[Diagnostic], bool]:
def severity_included(diagnostic: Diagnostic) -> bool:
return diagnostic_severity(diagnostic) <= max_severity
return severity_included
def is_severity_included(max_severity: int) -> Callable[[Diagnostic], bool]:
return lambda diagnostics: diagnostic_severity(diagnostic) <= max_severity

8 changes: 8 additions & 0 deletions plugin/core/url.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ def parse_uri(uri: str) -> Tuple[str, str]:
return parsed.scheme, uri


def unparse_uri(parsed_uri: Tuple[str, str]) -> str:
"""
Reverse of `parse_uri()`.
"""
scheme, path = parsed_uri
return filename_to_uri(path) if scheme == "file" else path


def _to_resource_uri(path: str, prefix: str) -> str:
"""
Terrible hacks from ST core leak into packages as well.
Expand Down
40 changes: 25 additions & 15 deletions plugin/core/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,34 +647,44 @@ def format_diagnostic_for_panel(diagnostic: Diagnostic) -> Tuple[str, Optional[i
When the last three elemenst are not optional, show an inline phantom
using the information given.
"""
formatted = [diagnostic_source(diagnostic)]
offset = None
href = None
code = diagnostic.get("code")
if code is not None:
code = str(code)
formatted.append(":")
code_description = diagnostic.get("codeDescription")
if code_description:
href = code_description["href"]
else:
formatted.append(code)
formatted, code, href = diagnostic_source_and_code(diagnostic)
lines = diagnostic["message"].splitlines() or [""]
# \u200B is the zero-width space
result = " {:>4}:{:<4}{:<8}{} \u200B{}".format(
diagnostic["range"]["start"]["line"] + 1,
diagnostic["range"]["start"]["character"] + 1,
format_severity(diagnostic_severity(diagnostic)),
lines[0],
"".join(formatted)
formatted
)
if href:
offset = len(result)
offset = len(result) if href else None
for line in itertools.islice(lines, 1, None):
result += "\n" + 18 * " " + line
return result, offset, code, href


def format_diagnostic_source_and_code(diagnostic: Diagnostic) -> str:
formatted, code, href = diagnostic_source_and_code(diagnostic)
if href is None or code is None:
return formatted
return formatted + code


def diagnostic_source_and_code(diagnostic: Diagnostic) -> Tuple[str, Optional[str], Optional[str]]:
formatted = [diagnostic_source(diagnostic)]
href = None
code = diagnostic.get("code")
if code is not None:
code = str(code)
formatted.append(":")
code_description = diagnostic.get("codeDescription")
if code_description:
href = code_description["href"]
else:
formatted.append(code)
return "".join(formatted), code, href


def location_to_human_readable(
config: ClientConfig,
base_dir: Optional[str],
Expand Down
6 changes: 5 additions & 1 deletion plugin/core/windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from .configurations import ConfigManager
from .configurations import WindowConfigManager
from .diagnostics import ensure_diagnostics_panel
from .diagnostics_manager import is_severity_included
from .logging import debug
from .logging import exception_log
from .message_request_handler import MessageRequestHandler
Expand All @@ -25,6 +26,7 @@
from .typing import Optional, Any, Dict, Deque, List, Generator, Tuple, Iterable, Sequence, Union
from .url import parse_uri
from .views import extract_variables
from .views import format_diagnostic_for_panel
from .views import make_link
from .workspace import ProjectFolders
from .workspace import sorted_workspace_folders
Expand Down Expand Up @@ -505,13 +507,15 @@ def update_diagnostics_panel_async(self) -> None:
listeners = list(self._listeners)
prephantoms = [] # type: List[Tuple[int, int, str, str]]
row = 0
max_severity = userprefs().diagnostics_panel_include_severity_level
contributions = OrderedDict(
) # type: OrderedDict[str, List[Tuple[str, Optional[int], Optional[str], Optional[str]]]]
for session in self._sessions:
local_errors, local_warnings = session.diagnostics_manager.sum_total_errors_and_warnings_async()
self.total_error_count += local_errors
self.total_warning_count += local_warnings
for path, contribution in session.diagnostics_manager.diagnostics_panel_contributions_async():
for (_, path), contribution in session.diagnostics_manager.filter_map_diagnostics_async(
is_severity_included(max_severity), lambda _, diagnostic: format_diagnostic_for_panel(diagnostic)):
seen = path in contributions
contributions.setdefault(path, []).extend(contribution)
if not seen:
Expand Down
Loading