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

Allow suppressing UI blocking dialogs #2546

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
76420cc
add 'show_ui_blocking_message' config
eugenesvk Nov 6, 2024
2a92fc6
allow replacing blocking dialog modals with a status+console message
eugenesvk Nov 6, 2024
81e3567
add 'show_ui_blocking_err_message' config
eugenesvk Nov 6, 2024
f552660
allow replacing blocking error dialog modals with a status+console me…
eugenesvk Nov 6, 2024
1bbcef4
add "see console"
eugenesvk Nov 6, 2024
c0c9ded
use window instead of sublime for message dialogs
eugenesvk Nov 6, 2024
3747b46
use window instead of sublime for error dialogs
eugenesvk Nov 6, 2024
74c0af3
remove ⚠️ for messages (leave ❗ for errors)
eugenesvk Nov 7, 2024
d78e139
combine two blocking options into one suppress_error_dialogs
eugenesvk Nov 7, 2024
96884ff
suppress dialogs by default
eugenesvk Nov 7, 2024
a2743ce
swap logging logick to reflect the settings change
eugenesvk Nov 7, 2024
39f6429
move setting to other
eugenesvk Nov 7, 2024
9d40852
use longer names
eugenesvk Nov 7, 2024
987ce91
add missing window arg
eugenesvk Nov 7, 2024
5b99040
accept optional sublime Window in notify functions
eugenesvk Nov 7, 2024
33ab71f
fix style
eugenesvk Nov 7, 2024
27aa180
remove comments
eugenesvk Nov 27, 2024
d61affa
update type/var name
eugenesvk Nov 27, 2024
726a781
return if no window
eugenesvk Nov 27, 2024
8595bf2
shorten status message to remove LSP
eugenesvk Nov 27, 2024
5ab0e73
moved notification logging functions to a separate file to avoid circ…
eugenesvk Nov 27, 2024
546a8df
consolidate error notification into 1 function
eugenesvk Nov 27, 2024
6bdb715
fix flake8 style
eugenesvk Nov 27, 2024
76a222e
add LSP prefix to console log messages
eugenesvk Nov 27, 2024
52d5ee7
make status message optional
eugenesvk Nov 27, 2024
89c4bd9
remove severity level from log messages
eugenesvk Nov 27, 2024
ac0c301
update default config
eugenesvk Nov 27, 2024
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
3 changes: 3 additions & 0 deletions LSP.sublime-settings
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,9 @@

// --- Other --------------------------------------------------------------------------

// Replace error popup windows with a console message and a short status notification.
"suppress_error_dialogs": false,

// Show symbol references in Sublime's quick panel instead of the output panel.
"show_references_in_quick_panel": true,

Expand Down
7 changes: 5 additions & 2 deletions plugin/code_actions.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from __future__ import annotations
from .core.logging_notify import notify_error
from .core.promise import Promise
from .core.protocol import CodeAction
from .core.protocol import CodeActionKind
Expand Down Expand Up @@ -352,7 +353,8 @@ def run_async() -> None:

def _handle_response_async(self, session_name: str, response: Any) -> None:
if isinstance(response, Error):
sublime.error_message(f"{session_name}: {str(response)}")
message = f"{session_name}: {str(response)}"
notify_error(self.view.window(), message)


# This command must be a WindowCommand in order to reliably hide corresponding menu entries when no view has focus.
Expand Down Expand Up @@ -414,7 +416,8 @@ def run_async(self, index: int, event: dict | None) -> None:

def _handle_response_async(self, session_name: str, response: Any) -> None:
if isinstance(response, Error):
sublime.error_message(f"{session_name}: {str(response)}")
message = f"{session_name}: {str(response)}"
notify_error(self.window, message)

def _is_cache_valid(self, event: dict | None) -> bool:
view = self.view
Expand Down
18 changes: 18 additions & 0 deletions plugin/core/logging_notify.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from __future__ import annotations
import sublime
from .settings import userprefs


def notify_error(window: sublime.Window | None, message: str, status_message: str | None = None) -> None:
"""Pick either of the 2 ways to show a user error notification message:
- via a detailed console message and a short status message
- via a blocking error modal dialog"""
if not window:
return
if status_message is None:
status_message = message
if userprefs().suppress_error_dialogs:
window.status_message(status_message)
print("LSP: " + message)
else:
sublime.error_message(message)
2 changes: 2 additions & 0 deletions plugin/core/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ class Settings:
show_code_actions_in_hover = cast(bool, None)
show_diagnostics_annotations_severity_level = cast(int, None)
show_diagnostics_count_in_view_status = cast(bool, None)
suppress_error_dialogs = cast(bool, None)
show_multiline_diagnostics_highlights = cast(bool, None)
show_multiline_document_highlights = cast(bool, None)
show_diagnostics_in_view_status = cast(bool, None)
Expand Down Expand Up @@ -278,6 +279,7 @@ def r(name: str, default: bool | int | str | list | dict) -> None:
r("show_code_actions_in_hover", True)
r("show_diagnostics_annotations_severity_level", 0)
r("show_diagnostics_count_in_view_status", False)
r("suppress_error_dialogs", True)
r("show_diagnostics_in_view_status", True)
r("show_multiline_diagnostics_highlights", True)
r("show_multiline_document_highlights", True)
Expand Down
4 changes: 3 additions & 1 deletion plugin/core/windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .diagnostics_storage import is_severity_included
from .logging import debug
from .logging import exception_log
from .logging_notify import notify_error
from .message_request_handler import MessageRequestHandler
from .panels import LOG_LINES_LIMIT_SETTING_NAME
from .panels import MAX_LOG_LINES_LIMIT_OFF
Expand Down Expand Up @@ -295,12 +296,13 @@ def start_async(self, config: ClientConfig, initiating_view: sublime.View) -> No
"Re-enable by running \"LSP: Enable Language Server In Project\" from the Command Palette.",
"\n\n--- Error: ---\n{1}"
)).format(config.name, str(e))
status = f"Failed to start {config.name}… See console"
exception_log(f"Unable to start subprocess for {config.name}", e)
if isinstance(e, CalledProcessError):
print("Server output:\n{}".format(e.output.decode('utf-8', 'replace')))
self._config_manager.disable_config(config.name, only_for_session=True)
config.erase_view_status(initiating_view)
sublime.message_dialog(message)
notify_error(self._window, message, status)
# Continue with handling pending listeners
self._new_session = None
sublime.set_timeout_async(self._dequeue_listener_async)
Expand Down
11 changes: 7 additions & 4 deletions plugin/core/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from .types import matches_pattern
from .types import sublime_pattern_to_glob
from .url import filename_to_uri
from .logging_notify import notify_error
from typing import Any
import sublime
import os
Expand Down Expand Up @@ -146,8 +147,9 @@ def enable_in_project(window: sublime.Window, config_name: str) -> None:
project_client_settings['enabled'] = True
window.set_project_data(project_data)
else:
sublime.message_dialog(
f"Can't enable {config_name} in the current workspace. Ensure that the project is saved first.")
message = f"Can't enable {config_name} in the current workspace. Ensure that the project is saved first."
status = f"Can't enable {config_name} in this workspace… See console"
notify_error(window, message, status)


def disable_in_project(window: sublime.Window, config_name: str) -> None:
Expand All @@ -159,5 +161,6 @@ def disable_in_project(window: sublime.Window, config_name: str) -> None:
project_client_settings['enabled'] = False
window.set_project_data(project_data)
else:
sublime.message_dialog(
f"Can't disable {config_name} in the current workspace. Ensure that the project is saved first.")
message = f"Can't disable {config_name} in the current workspace. Ensure that the project is saved first."
status = f"Can't enable {config_name} in this workspace… See console"
notify_error(window, message, status)
5 changes: 4 additions & 1 deletion plugin/execute_command.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from __future__ import annotations
from .core.logging_notify import notify_error
from .core.protocol import Error
from .core.protocol import ExecuteCommandParams
from .core.registry import LspTextCommand
Expand Down Expand Up @@ -58,7 +59,9 @@ def handle_error_async(self, error: Error, command_name: str) -> None:
:param error: The Error object.
:param command_name: The name of the command that was executed.
"""
sublime.message_dialog(f"command {command_name} failed. Reason: {str(error)}")
message = f"command {command_name} failed. Reason: {str(error)}"
status = f"{command_name} failed… See console"
notify_error(self.view.window(), message, status)

def _expand_variables(self, command_args: list[Any]) -> list[Any]:
view = self.view
Expand Down
7 changes: 5 additions & 2 deletions plugin/rename.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from .core.edit import parse_range
from .core.edit import parse_workspace_edit
from .core.edit import WorkspaceChanges
from .core.logging_notify import notify_error
from .core.protocol import PrepareRenameParams
from .core.protocol import PrepareRenameResult
from .core.protocol import Range
Expand Down Expand Up @@ -211,7 +212,8 @@ def _on_rename_result_async(self, session: Session, response: WorkspaceEdit | No

def _on_prepare_result(self, pos: int, session_name: str | None, response: PrepareRenameResult | None) -> None:
if response is None:
sublime.error_message("The current selection cannot be renamed")
message = "The current selection cannot be renamed"
notify_error(self.view.window(), message)
return
if is_range_response(response):
r = range_to_region(response, self.view)
Expand All @@ -226,7 +228,8 @@ def _on_prepare_result(self, pos: int, session_name: str | None, response: Prepa
self.view.run_command("lsp_symbol_rename", args)

def _on_prepare_error(self, error: Any) -> None:
sublime.error_message("Rename error: {}".format(error["message"]))
message = "Rename error: {}".format(error["message"])
notify_error(self.view.window(), message)

def _get_relative_path(self, file_path: str) -> str:
wm = windows.lookup(self.view.window())
Expand Down
27 changes: 19 additions & 8 deletions plugin/tooling.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations
from .core.css import css
from .core.logging import debug
from .core.logging_notify import notify_error
from .core.registry import windows
from .core.sessions import get_plugin
from .core.transports import create_transport
Expand Down Expand Up @@ -130,15 +131,19 @@ def run(self, base_package_name: str) -> None:
try:
urllib.parse.urlparse(base_url)
except Exception:
sublime.error_message("The clipboard content must be a URL to a package.json file.")
message = "The clipboard content must be a URL to a package.json file."
status = "Clipboard must be a URL to package.json"
notify_error(sublime.active_window(), message, status)
return
if not base_url.endswith("package.json"):
sublime.error_message("URL must end with 'package.json'")
message = "URL must end with 'package.json'"
notify_error(sublime.active_window(), message)
return
try:
package = json.loads(urllib.request.urlopen(base_url).read().decode("utf-8"))
except Exception as ex:
sublime.error_message(f'Unable to load "{base_url}": {ex}')
message = f'Unable to load "{base_url}": {ex}'
notify_error(sublime.active_window(), message)
return

# There might be a translations file as well.
Expand All @@ -150,11 +155,13 @@ def run(self, base_package_name: str) -> None:

contributes = package.get("contributes")
if not isinstance(contributes, dict):
sublime.error_message('No "contributes" key found!')
message = 'No "contributes" key found!'
notify_error(sublime.active_window(), message)
return
configuration = contributes.get("configuration")
if not isinstance(configuration, dict) and not isinstance(configuration, list):
sublime.error_message('No "contributes.configuration" key found!')
message = 'No "contributes.configuration" key found!'
notify_error(sublime.active_window(), message)
return
if isinstance(configuration, dict):
properties = configuration.get("properties")
Expand All @@ -163,7 +170,8 @@ def run(self, base_package_name: str) -> None:
for configuration_item in configuration:
properties.update(configuration_item.get("properties"))
if not isinstance(properties, dict):
sublime.error_message('No "contributes.configuration.properties" key found!')
message = 'No "contributes.configuration.properties" key found!'
notify_error(sublime.active_window(), message)
return

# Process each key-value pair of the server settings.
Expand Down Expand Up @@ -303,7 +311,8 @@ def run(self) -> None:
return
view = wm.window.active_view()
if not view:
sublime.message_dialog('Troubleshooting must be run with a file opened')
message = 'Troubleshooting must be run with a file opened'
notify_error(self.window, message)
return
active_view = view
configs = wm.get_config_manager().get_configs()
Expand Down Expand Up @@ -457,7 +466,9 @@ def run(self, edit: sublime.Edit) -> None:
return
listener = wm.listener_for_view(self.view)
if not listener or not any(listener.session_views_async()):
sublime.error_message("There is no language server running for this view.")
message = "There is no language server running for this view."
status = "No language server for this view"
notify_error(wm.window, message, status)
return
v = wm.window.new_file()
v.set_scratch(True)
Expand Down
5 changes: 5 additions & 0 deletions sublime-package.json
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,11 @@
"default": false,
"markdownDescription": "Show errors and warnings count in the status bar."
},
"suppress_error_dialogs": {
"type": "boolean",
"default": false,
Copy link
Member

Choose a reason for hiding this comment

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

This is true in the settings file.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

Copy link
Author

Choose a reason for hiding this comment

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

I would just print the messages to the status bar and console.

So let's revert it back to true then here and in the settings

Copy link
Member

Choose a reason for hiding this comment

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

Yeah my intention was to fix this line, and not to change the value in the settings. But then I would think that probably nobody will change this setting anyway, so I would just leave it out. We can't add a new setting for each tiny aspect of this package's behavior. I don't expect that people will care very much when the error dialogs went away and I don't think they will complain about that. I recently also removed another of these dialogs in #2528 (which is not covered in this PR) and I think it is the right thing to do. Though if the other contributors think differently, I'd be okay if my opinion gets overruled. And I would let @rchl decide whether the error messages in tooling.py should keep their dialog windows or not, because iirc he implemented that (and as I already wrote, this troubleshooting command is not a common action during editing anyway).

"markdownDescription": "Replace error popup windows with a console message and a short status notification."
},
"lsp_format_on_paste": {
"$ref": "sublime://settings/LSP#/definitions/lsp_format_on_paste"
},
Expand Down