From be4ef8ecbb8b91c10db04ff8866d2b68e36d6c0e Mon Sep 17 00:00:00 2001 From: Raoul Wols Date: Sat, 5 Jun 2021 11:33:46 +0200 Subject: [PATCH] Refine diagnostics presentation (#1710) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the diagnostic spans one line: - Draw errors and warnings as squigglies - Draw infos and hints as stippled If the diagnostic spans more than one line: always draw as a box. Co-authored-by: Rafał Chłodnicki --- LSP.sublime-settings | 10 ++--- docs/src/customization.md | 47 ++++++++++------------ plugin/core/sessions.py | 4 +- plugin/core/types.py | 30 ++++----------- plugin/core/views.py | 12 +++--- plugin/documents.py | 5 ++- plugin/session_buffer.py | 20 +++++----- plugin/session_view.py | 62 ++++++++++++++++-------------- sublime-package.json | 17 ++++---- tests/test_server_notifications.py | 14 +++---- 10 files changed, 103 insertions(+), 118 deletions(-) diff --git a/LSP.sublime-settings b/LSP.sublime-settings index a0fb74054..78e178edc 100644 --- a/LSP.sublime-settings +++ b/LSP.sublime-settings @@ -78,11 +78,6 @@ // See also: "diagnostics_delay_ms". "diagnostics_additional_delay_auto_complete_ms": 0, - // Highlighting style of code diagnostics. - // Valid values are "fill", "box", "underline", "stippled", "squiggly" or "". - // When set to the empty string (""), no diagnostics are shown in-line. - "diagnostics_highlight_style": "squiggly", - // Highlighting style of "highlights": accentuating nearby text entities that // are related to the one under your cursor. // Valid values are "fill", "underline", or "". @@ -104,6 +99,11 @@ // Show code actions in hover popup if available "show_code_actions_in_hover": true, + // Show diagnostics (error/warning squigglies) in the view. When disabled, + // gutter markers are still drawn, unless "diagnostics_gutter_marker" is + // set to "". + "show_diagnostics_highlights": true, + // Show symbol action links in hover popup if available "show_symbol_action_links": true, diff --git a/docs/src/customization.md b/docs/src/customization.md index d1dfad7f0..527b593b1 100644 --- a/docs/src/customization.md +++ b/docs/src/customization.md @@ -37,15 +37,14 @@ The following tables give an overview about the scope names used by LSP. ### Diagnostics -| scope | DiagnosticSeverity | description | -| ----- | ------------------ | ----------- | -| `markup.error.lsp` | Error | Reports an error | -| `markup.warning.lsp` | Warning | Reports a warning | -| `markup.info.lsp` | Information | Reports an information | -| `markup.info.hint.lsp` | Hint | Reports a hint | +| scope | DiagnosticSeverity | description | drawn as +| ----- | ------------------ | ----------- | -------- +| `markup.error.lsp` | Error | Reports an error | Squiggly underlines +| `markup.warning.lsp` | Warning | Reports a warning | Squiggly underlines +| `markup.info.lsp` | Information | Reports an information | Stippled underlines +| `markup.info.hint.lsp` | Hint | Reports a hint | Stippled underlines -!!! note - If `diagnostics_highlight_style` is set to "fill" in the LSP settings, the highlighting color can be controlled via the "background" color from a color scheme rule for the listed scopes. +When the region of the diagnostic spans more than one line, the diagnostic is always drawn as a box. Diagnostics will also optionally include the following scopes: @@ -54,27 +53,23 @@ Diagnostics will also optionally include the following scopes: | `markup.unnecessary.lsp` | Unnecessary | Unused or unnecessary code | | `markup.deprecated.lsp` | Deprecated | Deprecated or obsolete code | -!!! note - Regions created for those scopes don't follow the `diagnostics_highlight_style` setting and instead always use the "fill" style. - - Those scopes can be used to, for example, gray-out the text color of unused code, if the server supports that. +Those scopes can be used to, for example, gray-out the text color of unused code, if the server supports that. - For example, to add a custom rule for `Mariana` color scheme, select `UI: Customize Color Scheme` from the Command Palette and add the following rule: +For example, to add a custom rule for `Mariana` color scheme, select `UI: Customize Color Scheme` from the Command Palette and add the following rule: - ```json - { - "rules": [ - { - "scope": "markup.unnecessary.lsp", - "foreground": "color(rgb(255, 255, 255) alpha(0.4))", - "background": "color(var(blue3) alpha(0.9))" - } - ] - } - ``` - - The color scheme rule only works if the "background" color is different from the global background of the scheme. So for other color schemes, ideally pick a background color that is as close as possible, but marginally different from the original background. +```json +{ + "rules": [ + { + "scope": "markup.unnecessary.lsp", + "foreground": "color(rgb(255, 255, 255) alpha(0.4))", + "background": "color(var(blue3) alpha(0.9))" + } + ] +} +``` +The color scheme rule only works if the "background" color is different from the global background of the scheme. So for other color schemes, ideally pick a background color that is as close as possible, but marginally different from the original background. ### Signature Help diff --git a/plugin/core/sessions.py b/plugin/core/sessions.py index e63f42a0f..197d86b7d 100644 --- a/plugin/core/sessions.py +++ b/plugin/core/sessions.py @@ -328,7 +328,7 @@ def has_capability_async(self, capability_path: str) -> bool: def shutdown_async(self) -> None: ... - def present_diagnostics_async(self, flags: int) -> None: + def present_diagnostics_async(self) -> None: ... def on_request_started_async(self, request_id: int, request: Request) -> None: @@ -365,7 +365,7 @@ def unregister_capability_async( ) -> None: ... - def on_diagnostics_async(self, diagnostics: List[Diagnostic], version: Optional[int]) -> None: + def on_diagnostics_async(self, raw_diagnostics: List[Diagnostic], version: Optional[int]) -> None: ... diff --git a/plugin/core/types.py b/plugin/core/types.py index 30923023a..27d33cf03 100644 --- a/plugin/core/types.py +++ b/plugin/core/types.py @@ -69,23 +69,6 @@ def run() -> None: runner(run, timeout_ms) -def _settings_style_to_add_regions_flag(style: str) -> int: - flags = 0 - if style == "fill": - flags = sublime.DRAW_NO_OUTLINE - elif style == "box": - flags = sublime.DRAW_NO_FILL - else: - flags = sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE - if style == "underline": - flags |= sublime.DRAW_SOLID_UNDERLINE - elif style == "stippled": - flags |= sublime.DRAW_STIPPLED_UNDERLINE - elif style == "squiggly": - flags |= sublime.DRAW_SQUIGGLY_UNDERLINE - return flags - - class SettingsRegistration: __slots__ = ("_settings",) @@ -151,7 +134,6 @@ class Settings: diagnostics_additional_delay_auto_complete_ms = None # type: int diagnostics_delay_ms = None # type: int diagnostics_gutter_marker = None # type: str - diagnostics_highlight_style = None # type: str diagnostics_panel_include_severity_level = None # type: int disabled_capabilities = None # type: List[str] document_highlight_style = None # type: str @@ -168,6 +150,7 @@ class Settings: show_code_actions = None # type: bool show_code_actions_in_hover = None # type: bool show_diagnostics_count_in_view_status = None # type: bool + show_diagnostics_highlights = None # type: bool show_diagnostics_in_view_status = None # type: bool show_diagnostics_panel_on_save = None # type: int show_diagnostics_severity_level = None # type: int @@ -188,7 +171,6 @@ def r(name: str, default: Union[bool, int, str, list, dict]) -> None: r("diagnostics_additional_delay_auto_complete_ms", 0) r("diagnostics_delay_ms", 0) r("diagnostics_gutter_marker", "dot") - r("diagnostics_highlight_style", "underline") r("diagnostics_panel_include_severity_level", 4) r("disabled_capabilities", []) r("document_highlight_style", "underline") @@ -203,6 +185,7 @@ def r(name: str, default: Union[bool, int, str, list, dict]) -> None: r("show_code_actions_in_hover", True) r("show_diagnostics_count_in_view_status", False) r("show_diagnostics_in_view_status", True) + r("show_diagnostics_highlights", True) r("show_diagnostics_panel_on_save", 2) r("show_diagnostics_severity_level", 2) r("show_references_in_quick_panel", False) @@ -236,6 +219,12 @@ def r(name: str, default: Union[bool, int, str, list, dict]) -> None: r("inhibit_snippet_completions", False) r("inhibit_word_completions", True) + # Backwards-compatible with "diagnostics_highlight_style" + diagnostics_highlight_style = s.get("diagnostics_highlight_style") + if isinstance(diagnostics_highlight_style, str): + if not diagnostics_highlight_style: + self.show_diagnostics_highlights = False + set_debug_logging(self.log_debug) def document_highlight_style_region_flags(self) -> Tuple[int, int]: @@ -244,9 +233,6 @@ def document_highlight_style_region_flags(self) -> Tuple[int, int]: else: return sublime.DRAW_NO_FILL, sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE | sublime.DRAW_SOLID_UNDERLINE - def diagnostics_highlight_style_to_add_regions_flag(self) -> int: - return _settings_style_to_add_regions_flag(self.diagnostics_highlight_style) - class ClientStates: STARTING = 0 diff --git a/plugin/core/views.py b/plugin/core/views.py index 0e3ccc408..6a3c77702 100644 --- a/plugin/core/views.py +++ b/plugin/core/views.py @@ -30,12 +30,12 @@ import tempfile DIAGNOSTIC_SEVERITY = [ - # Kind CSS class Scope for color Icon resource - ("error", "errors", "region.redish markup.error.lsp", "Packages/LSP/icons/error.png"), - ("warning", "warnings", "region.yellowish markup.warning.lsp", "Packages/LSP/icons/warning.png"), - ("info", "info", "region.bluish markup.info.lsp", "Packages/LSP/icons/info.png"), - ("hint", "hints", "region.bluish markup.info.hint.lsp", "Packages/LSP/icons/info.png"), -] + # Kind CSS class Scope for color Icon resource add_regions flags for single-line diagnostic multi-line diagnostic # noqa: E501 + ("error", "errors", "region.redish markup.error.lsp", "Packages/LSP/icons/error.png", sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE | sublime.DRAW_SQUIGGLY_UNDERLINE, sublime.DRAW_NO_FILL), # noqa: E501 + ("warning", "warnings", "region.yellowish markup.warning.lsp", "Packages/LSP/icons/warning.png", sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE | sublime.DRAW_SQUIGGLY_UNDERLINE, sublime.DRAW_NO_FILL), # noqa: E501 + ("info", "info", "region.bluish markup.info.lsp", "Packages/LSP/icons/info.png", sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE | sublime.DRAW_STIPPLED_UNDERLINE, sublime.DRAW_NO_FILL), # noqa: E501 + ("hint", "hints", "region.bluish markup.info.hint.lsp", "Packages/LSP/icons/info.png", sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE | sublime.DRAW_STIPPLED_UNDERLINE, sublime.DRAW_NO_FILL), # noqa: E501 +] # type: List[Tuple[str, str, str, str, int, int]] # The scope names mainly come from http://www.sublimetext.com/docs/3/scope_naming.html SYMBOL_KINDS = [ diff --git a/plugin/documents.py b/plugin/documents.py index 7c73b8ca7..85aadd684 100644 --- a/plugin/documents.py +++ b/plugin/documents.py @@ -226,7 +226,10 @@ def diagnostics_panel_contribution_async(self) -> List[Tuple[str, Optional[int], # Sort by severity for severity in range(1, len(DIAGNOSTIC_SEVERITY) + 1): for sb in self.session_buffers_async(): - data = sb.data_per_severity.get(severity) + data = sb.data_per_severity.get((severity, False)) + if data: + result.extend(data.panel_contribution) + data = sb.data_per_severity.get((severity, True)) if data: result.extend(data.panel_contribution) # sort the result by asc line number diff --git a/plugin/session_buffer.py b/plugin/session_buffer.py index 7b2153899..98ce84164 100644 --- a/plugin/session_buffer.py +++ b/plugin/session_buffer.py @@ -46,7 +46,7 @@ def __init__(self, severity: int) -> None: self.regions_with_tag = {} # type: Dict[int, List[sublime.Region]] self.annotations = [] # type: List[str] self.panel_contribution = [] # type: List[Tuple[str, Optional[int], Optional[str], Optional[str]]] - _, _, self.scope, self.icon = DIAGNOSTIC_SEVERITY[severity - 1] + _, _, self.scope, self.icon, _, _ = DIAGNOSTIC_SEVERITY[severity - 1] if userprefs().diagnostics_gutter_marker != "sign": self.icon = userprefs().diagnostics_gutter_marker @@ -76,7 +76,7 @@ def __init__(self, session_view: SessionViewProtocol, buffer_id: int, language_i self.id = buffer_id self.pending_changes = None # type: Optional[PendingChanges] self.diagnostics = [] # type: List[Tuple[Diagnostic, sublime.Region]] - self.data_per_severity = {} # type: Dict[int, DiagnosticSeverityData] + self.data_per_severity = {} # type: Dict[Tuple[int, bool], DiagnosticSeverityData] self.diagnostics_version = -1 self.diagnostics_flags = 0 self.diagnostics_are_visible = False @@ -245,7 +245,7 @@ def some_view(self) -> Optional[sublime.View]: return None def on_diagnostics_async(self, raw_diagnostics: List[Diagnostic], version: Optional[int]) -> None: - data_per_severity = {} # type: Dict[int, DiagnosticSeverityData] + data_per_severity = {} # type: Dict[Tuple[int, bool], DiagnosticSeverityData] total_errors = 0 total_warnings = 0 should_show_diagnostics_panel = False @@ -259,12 +259,13 @@ def on_diagnostics_async(self, raw_diagnostics: List[Diagnostic], version: Optio diagnostics_version = version diagnostics = [] # type: List[Tuple[Diagnostic, sublime.Region]] for diagnostic in raw_diagnostics: + region = range_to_region(Range.from_lsp(diagnostic["range"]), view) severity = diagnostic_severity(diagnostic) - data = data_per_severity.get(severity) + key = (severity, len(view.split_by_newlines(region)) > 1) + data = data_per_severity.get(key) if data is None: data = DiagnosticSeverityData(severity) - data_per_severity[severity] = data - region = range_to_region(Range.from_lsp(diagnostic["range"]), view) + data_per_severity[key] = data tags = diagnostic.get('tags', []) if tags: for tag in tags: @@ -293,7 +294,7 @@ def _publish_diagnostics_to_session_views( self, diagnostics_version: int, diagnostics: List[Tuple[Diagnostic, sublime.Region]], - data_per_severity: Dict[int, DiagnosticSeverityData], + data_per_severity: Dict[Tuple[int, bool], DiagnosticSeverityData], total_errors: int, total_warnings: int, should_show_diagnostics_panel: bool @@ -336,7 +337,7 @@ def _present_diagnostics_async( self, diagnostics_version: int, diagnostics: List[Tuple[Diagnostic, sublime.Region]], - data_per_severity: Dict[int, DiagnosticSeverityData], + data_per_severity: Dict[Tuple[int, bool], DiagnosticSeverityData], total_errors: int, total_warnings: int, should_show_diagnostics_panel: bool @@ -348,9 +349,8 @@ def _present_diagnostics_async( self.total_errors = total_errors self.total_warnings = total_warnings self.should_show_diagnostics_panel = should_show_diagnostics_panel - flags = userprefs().diagnostics_highlight_style_to_add_regions_flag() for sv in self.session_views: - sv.present_diagnostics_async(flags) + sv.present_diagnostics_async() mgr = self.session.manager() if mgr: mgr.update_diagnostics_panel_async() diff --git a/plugin/session_view.py b/plugin/session_view.py index e2f649250..ad09c3ae5 100644 --- a/plugin/session_view.py +++ b/plugin/session_view.py @@ -68,7 +68,8 @@ def __del__(self) -> None: self.session.unregister_session_view_async(self) self.session.config.erase_view_status(self.view) for severity in reversed(range(1, len(DIAGNOSTIC_SEVERITY) + 1)): - self.view.erase_regions(self.diagnostics_key(severity)) + self.view.erase_regions(self.diagnostics_key(severity, False)) + self.view.erase_regions(self.diagnostics_key(severity, True)) def _clear_auto_complete_triggers(self, settings: sublime.Settings) -> None: '''Remove all of our modifications to the view's "auto_complete_triggers"''' @@ -159,10 +160,10 @@ def on_capability_added_async(self, registration_id: str, capability_path: str, if listener: listener.on_code_lens_capability_registered_async() - def on_capability_removed_async(self, registration_id: str, discarded: Dict[str, Any]) -> None: - if self.HOVER_PROVIDER_KEY in discarded: + def on_capability_removed_async(self, registration_id: str, discarded_capabilities: Dict[str, Any]) -> None: + if self.HOVER_PROVIDER_KEY in discarded_capabilities: self._decrement_hover_count() - elif self.COMPLETION_PROVIDER_KEY in discarded: + elif self.COMPLETION_PROVIDER_KEY in discarded_capabilities: self._unregister_auto_complete_triggers(registration_id) def has_capability_async(self, capability_path: str) -> bool: @@ -173,8 +174,8 @@ def shutdown_async(self) -> None: if listener: listener.on_session_shutdown_async(self.session) - def diagnostics_key(self, severity: int) -> str: - return "lsp{}d{}".format(self.session.config.name, severity) + def diagnostics_key(self, severity: int, multiline: bool) -> str: + return "lsp{}d{}{}".format(self.session.config.name, "m" if multiline else "s", severity) def diagnostics_tag_scope(self, tag: int) -> Optional[str]: for k, v in DiagnosticTag.__dict__.items(): @@ -182,33 +183,36 @@ def diagnostics_tag_scope(self, tag: int) -> Optional[str]: return 'markup.{}.lsp'.format(k.lower()) return None - def present_diagnostics_async(self, flags: int) -> None: - data_per_severity = self.session_buffer.data_per_severity - for severity in reversed(range(1, len(DIAGNOSTIC_SEVERITY) + 1)): - key = self.diagnostics_key(severity) - key_tags = {tag: '{}_tags_{}'.format(key, tag) for tag in DIAGNOSTIC_TAG_VALUES} - for key_tag in key_tags.values(): - self.view.erase_regions(key_tag) - data = data_per_severity.get(severity) - if data is None: - self.view.erase_regions(key) - elif ((severity <= userprefs().show_diagnostics_severity_level) and - (data.icon or flags != (sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE))): - non_tag_regions = data.regions - for tag, regions in data.regions_with_tag.items(): - tag_scope = self.diagnostics_tag_scope(tag) - # Trick to only add tag regions if there is a corresponding color scheme scope defined. - if tag_scope and 'background' in self.view.style_for_scope(tag_scope): - self.view.add_regions(key_tags[tag], regions, tag_scope, flags=sublime.DRAW_NO_OUTLINE) - else: - non_tag_regions.extend(regions) - self.view.add_regions(key, non_tag_regions, data.scope, data.icon, flags | sublime.DRAW_EMPTY) - else: - self.view.erase_regions(key) + def present_diagnostics_async(self) -> None: + flags = 0 if userprefs().show_diagnostics_highlights else sublime.DRAW_NO_FILL | sublime.DRAW_NO_OUTLINE + level = userprefs().show_diagnostics_severity_level + for sev in reversed(range(1, len(DIAGNOSTIC_SEVERITY) + 1)): + self._draw_diagnostics(sev, level, DIAGNOSTIC_SEVERITY[sev - 1][4] if flags == 0 else flags, False) + self._draw_diagnostics(sev, level, DIAGNOSTIC_SEVERITY[sev - 1][5] if flags == 0 else flags, True) listener = self.listener() if listener: listener.on_diagnostics_updated_async() + def _draw_diagnostics(self, severity: int, max_severity_level: int, flags: int, multiline: bool) -> None: + key = self.diagnostics_key(severity, multiline) + key_tags = {tag: '{}_tags_{}'.format(key, tag) for tag in DIAGNOSTIC_TAG_VALUES} + for key_tag in key_tags.values(): + self.view.erase_regions(key_tag) + data = self.session_buffer.data_per_severity.get((severity, multiline)) + # TODO: Why do we have this data.icon check? + if data and data.icon and severity <= max_severity_level: + non_tag_regions = data.regions + for tag, regions in data.regions_with_tag.items(): + tag_scope = self.diagnostics_tag_scope(tag) + # Trick to only add tag regions if there is a corresponding color scheme scope defined. + if tag_scope and 'background' in self.view.style_for_scope(tag_scope): + self.view.add_regions(key_tags[tag], regions, tag_scope, flags=sublime.DRAW_NO_OUTLINE) + else: + non_tag_regions.extend(regions) + self.view.add_regions(key, non_tag_regions, data.scope, data.icon, flags | sublime.DRAW_EMPTY) + else: + self.view.erase_regions(key) + def on_request_started_async(self, request_id: int, request: Request) -> None: self.active_requests[request_id] = request if request.progress: diff --git a/sublime-package.json b/sublime-package.json index b63ac43bc..b7323f8b3 100644 --- a/sublime-package.json +++ b/sublime-package.json @@ -237,16 +237,8 @@ "markdownDescription": "Add an additional delay when the auto-complete widget is currently visible. Just like `\"diagnostics_delay_ms\"`, the unit is milliseconds. The total amount of delay would be `diagnostics_delay_ms + diagnostics_additional_delay_auto_complete_ms`." }, "diagnostics_highlight_style": { - "enum": [ - "fill", - "box", - "underline", - "stippled", - "squiggly", - "" - ], - "default": "squiggly", - "markdownDescription": "Highlighting style of code diagnostics. When set to the empty string (`\"\"`), no diagnostics are shown in-line." + "enum": [], + "deprecationMessage": "Use \"show_diagnostics_highlights\" instead", }, "document_highlight_style": { "enum": [ @@ -287,6 +279,11 @@ "default": true, "markdownDescription": "Show code actions in hover popup if available." }, + "show_diagnostics_highlights": { + "type": "boolean", + "default": true, + "markdownDescription": "Show diagnostics (error/warning squigglies) in the view. When disabled, gutter markers are still drawn, unless `\"diagnostics_gutter_marker\"` is set to `\"\"`." + }, "show_symbol_action_links": { "type": "boolean", "default": true, diff --git a/tests/test_server_notifications.py b/tests/test_server_notifications.py index 333dd27f5..eb9e5abaf 100644 --- a/tests/test_server_notifications.py +++ b/tests/test_server_notifications.py @@ -36,13 +36,13 @@ def test_publish_diagnostics(self) -> Generator: ] } # type: PublishDiagnosticsParams yield from self.await_client_notification("textDocument/publishDiagnostics", params) - yield lambda: len(self.view.get_regions("lspTESTd1")) > 0 - yield lambda: len(self.view.get_regions("lspTESTd2")) > 0 - yield lambda: len(self.view.get_regions("lspTESTd3")) > 0 - yield lambda: len(self.view.get_regions("lspTESTd3_tags")) == 0 - errors = self.view.get_regions("lspTESTd1") - warnings = self.view.get_regions("lspTESTd2") - info = self.view.get_regions("lspTESTd3") + yield lambda: len(self.view.get_regions("lspTESTds1")) > 0 + yield lambda: len(self.view.get_regions("lspTESTds2")) > 0 + yield lambda: len(self.view.get_regions("lspTESTds3")) > 0 + yield lambda: len(self.view.get_regions("lspTESTds3_tags")) == 0 + errors = self.view.get_regions("lspTESTds1") + warnings = self.view.get_regions("lspTESTds2") + info = self.view.get_regions("lspTESTds3") self.assertEqual(len(errors), 1) self.assertEqual(errors[0], sublime.Region(0, 1)) self.assertEqual(len(warnings), 1)