Refine diagnostics presentation (#1710)

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 <rchl2k@gmail.com>

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,
 

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
 

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:
         ...
 
 

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

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 = [

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

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()

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,42 +174,45 @@ 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():
             if v == tag:
                 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: