sublimelsp / LSP

Client implementation of the Language Server Protocol for Sublime Text
https://lsp.sublimetext.io/
MIT License
1.66k stars 183 forks source link

Allow suppressing UI blocking dialogs #2546

Open eugenesvk opened 2 weeks ago

eugenesvk commented 2 weeks ago

Add 2 user configuration options that would allow to suppress the attention-demanding UI blocking dialogs, instead redirecting warnings/errors to a status message and the full console message

Simple way to close https://github.com/sublimelsp/LSP/issues/2545 while preserving the current defaults

Though not all modals are affected since some are invoked by the lsp_utils dependency

Question: is it possible to make that dependency respect config options from this plugin? Or do dependencies "by design" ignore their clients?

jwortmann commented 2 weeks ago

Here is my opinion:

jwortmann commented 2 weeks ago

Question: is it possible to make that dependency respect config options from this plugin? Or do dependencies "by design" ignore their clients?

Yes, should be possible. Just load settings via sublime.load_settings('LSP.sublime-settings').

eugenesvk commented 2 weeks ago

Updated the PR per the following:

  • I would probably be okay with such a change to remove (most of) the dialog windows, which don't require user input.

switched to suppress by default

  • if we want to make this configurable, it should be a single setting and use a better name. Maybe something like suppress_error_dialogs. No need to distinguish between message_dialog and error_dialog imo. But I would probably favor not to add a setting at all for this, unless you want to implement a "don't show this dialog again" checkbox/button.

Left the setting given that it's suppressed by default, also re the extra button: Sublime still hasn't learned to properly roundtrip user configs, and this button would change files in the User folder, thus breaking subtle formatting, so I'd just leave it in the currently less convenient format

  • the status text should give a reference to the console, like "Failed to start LSP-json - see console for details"

Added a shorter text, otherwise it might not fit

  • status message should use the Window.status_message method if possible, not sublime.status_message. And there are likely a few more things to clean up in the code, e.g. don't use local imports. But I haven't looked too closely yet.

switched

  • I don't really like emojis as ⚠️ in the status bar or console

They address @rchl concern of users not noticing notifications, ⚠️ make them stand out. I don't mind either way, so removed ⚠️, but left ❗ as I presume those are supposed to still be differentiated somehow

rchl commented 2 weeks ago
  • I don't really like emojis as ⚠️ in the status bar or console

They address @rchl concert of users not noticing notifications, ⚠️ make them stand out. I don't mind either way, so removed ⚠️, but left ❗ as I presume those are supposed to still be differentiated somehow

I do think that there will be a good portion of users that will not notice something showing up in the status field for a few seconds. Or if they do notice something changing, they might not realize what has changed (if the status field is busy).

So as I said, I don't think that status messages are a good solution in general.

I suppose when user is doing an explicit action then he/she might be more looking for some feedback/result and might pay more attention.

If the user doesn't expect anything then it might be easy to miss status change completely.

eugenesvk commented 2 weeks ago

After checking the manually shortened messages, there is actually no ❗ left, only in the short template when the only thing you see is ❗LSP: see console log… instead of any substatntive message, and since you both don't like them won't adde them to the actual status error messages

I do think that there will be a good portion of users that will not notice something showing up in the status field for a few seconds

Have you actually tried it? It's harder to miss colored ❗ motion in your vision periphery

So as I said, I don't think that status messages are a good solution in general.

By the way, for very important messages you could also use these popups, that's in user's focus (at the cursor) (and of course it could fit the whole message)

not