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

Disable blocking popup modal errors by default #2545

Open
eugenesvk opened this issue Nov 3, 2024 · 6 comments · May be fixed by #2546
Open

Disable blocking popup modal errors by default #2545

eugenesvk opened this issue Nov 3, 2024 · 6 comments · May be fixed by #2546

Comments

@eugenesvk
Copy link

eugenesvk commented Nov 3, 2024

Is your feature request related to a problem? Please describe.

After debugging some packages and adding a few to the list of ignored packages I get an infninite notification window

infinite mean that as soon as I close one, another one immediately pops up, so I can't even quit the editor properly!!! (if spam the close button fast enough, I am able to do)

And even outside of such pathological cases I don't care about a failing JSON parsing enough to even have a full UI blocking modal thrown in my face, so please disable it by default.

Describe the solution you'd like

UI blocking operations on errors are disabled by default and people who like them can toggle them on.

Instead you could show an error in the status bar, and even add some ❗❗❗ symbols to make it stand out but without having to block the UI

Describe alternatives you've considered

Maybe there is a way to auto-close these windows after some time and have a "debounce" to not show 10 errors in 10 seconds, but it seems more complicated than simply using a different mechanism

Additional context

That notification modal window:

lsp-json

(I think this is a generic LSP window, but not sure, so sorry if it's LSP-json-package specific)

@rchl
Copy link
Member

rchl commented Nov 5, 2024

There are different kind of errors and for some a modal dialog works better than the other options. Ideally ST would have something like a permanent popup that would stay until dismissed manually but it doesn't have that so I don't really see a better option for errors like when the server fails to start.

Status field doesn't really work well for those kind of errors. For many reasons... For example it doesn't have that much space to work with, it's not possible to trigger any action on clicking on it and it's not obvious when such error should be cleared.

Ideally, errors related to server failing to start should be very rare.

Infinite errors is a bug and should be fixed but we need some steps to reproduce those.

@eugenesvk
Copy link
Author

for some a modal dialog works better

Which ones? You could also have a different default for those, and let the users replace with less intrusive error type via a separate config

permanent popup that would stay until dismissed manually

But that's exactly what modal is! It's just that you open a new popup every time the old one is closed.

I don't really see a better option

The better option is to have user configuration to not show this dialog in the first place. It's a simple notification that requires no useful action from the user, and re. your other concerns:

  • there is an infinite amount of space in the console
  • and you can have user configurable timer after which the error goes away
  • pressing "OK" to acknowledge an error isn't a useful action

Ideally, errors related to server failing to start should be very rare.

Ok, but unfortunately the real world isn't ideal. Another case of wasted UI blocks I've had with LSP is that I've simply put off setting up a server to a local node, so got a modal on every ST startup (asking to download node, I think?). Again, something where a simple status message I could easily ignore until I can get to it would've been just fine

@rchl
Copy link
Member

rchl commented Nov 6, 2024

permanent popup that would stay until dismissed manually

But that's exactly what modal is! It's just that you open a new popup every time the old one is closed.

Not talking about blocking modal but a toast like notification in the corner that would not go away on its own.

Another case of wasted UI blocks I've had with LSP is that I've simply put off setting up a server to a local node, so got a modal on every ST startup (asking to download node, I think?). Again, something where a simple status message I could easily ignore until I can get to it would've been just fine

That dialog provides actionable buttons. Clicking on one of the buttons installs Node and allows the server to start. How else you imagine the flow should look like? Some status message that in a very little space tells the user to do what? Go to the console and find instructions there? It will certainly not be a user friendly approach, especially for new users.

@eugenesvk eugenesvk linked a pull request Nov 6, 2024 that will close this issue
@eugenesvk
Copy link
Author

toast like notification in the corner that would not go away on its own.

Ok, wouldn't want that either. I think the fundamental difference is that you think that these errors are very important and demand immediate user attention while I do not and would like to be able to configure that demand. In many cases like editing sublime config JSONs it would be a non-issue if LSP is broken for a long while.

If you don't want to change the attention-needing defaults, what about having an optional config that would replace all these modal errors with a console log instead + a short status message?
See this PR

Clicking on one of the buttons installs Node and allows the server to start.

Which I don't need to do right away and definitely don't need to be block-reminded of on every restart of Sublime.

How else you imagine the flow should look like? Some status message that in a very little space tells the user to do what? Go to the console and find instructions there?

You could combine the two: show a modal first, remember it was dismissed and later not show the same modal again (replace it with the status+console approach which tells which command you can run to install)
And the instructions could be "run this command", so a copy&paste away

It will certainly not be a user friendly approach

Yes, but that would still be friendlier than blocking new users from doing anything!

, especially for new users

That woud also be me, I just didn't need to drop everything and figure out how to set up paths to local binaries. Also as a later user I still don't want to do this after something broke that I'd eventually get to

@jwortmann
Copy link
Member

I must say that I'm also not a big fan of those modal dialog windows. But as pointed out just a message in the status bar wouldn't really work for those errors and to only print to the console would be even more pointless because nobody reads the console unless for debugging purpose. Furthermore, if there is an infinite loop creating these errors, you would then get infinite lines in the console, which means that the plugin host eats 100% of one of your CPU cores, and after some time your system might stall when it runs out of memory. So please provide exact reproduction steps so that we can fix that bug.

To consider a new setting, it would be nice if the dialog window had a checkbook you could tick that says something like "don't show this window again". Or perhaps you could think of a short label for an additional button in a yes/no/cancel dialog that would do that (not sure though if that would look and work really well).

The only feasible alternative I see would indeed be a notification system like in VSCode, but there is no API for such a UI element. Maybe someone should suggest it in the ST issue tracker if there isn't already a feature request for that. But it would probably take ages until it gets implemented, I'd they even care about it.

Though I believe to remember that there is also a dependency on package control which made it possible to show alerts/notifications via the operating system's native notification UI. Maybe that could be an alternative to use instead. But I don't know if that was only for Windows and if it is compatible with the 3.8 API.

@eugenesvk
Copy link
Author

But as pointed out just a message in the status bar wouldn't really work for those errors and to only print to the console would be even more pointless because nobody reads the console unless for debugging purpose.

Which is exactly what's needed - when I want to fix some LSP error, i.e., I want to debug it, I'll read the console! You still didn't even attempt to prove the high importance of these messages for every single user to warrant a UI blocking operation (otherwise you'd be fine with an user configurable default).

Furthermore, if there is an infinite loop creating these errors, you would then get infinite lines in the console, which means that the plugin host eats 100% of one of your CPU cores

Most likely I'd notice it and simply close the lsp-ed file or restart Sublime. Which is bad, but at the same time still beats the disruption of the infinite modal

So please provide exact reproduction steps so that we can fix that bug.

Sorry, but I've provided all the info I know.

(just an idea) Do you know what triggers the listener that invokes that error message? Could it be that when plugins got disabled a bunch of window/view settings changed and triggered a restart? (so maybe it wasn't really infinite, just a multitude)

it would be nice if the dialog window had a checkbook you could tick that says something like

Yes, it would be nice if Sublime allowed arbitrary UI modifications such as this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants