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

The formatting notification is too distracting #381

Closed
banacorn opened this issue Sep 7, 2020 · 9 comments
Closed

The formatting notification is too distracting #381

banacorn opened this issue Sep 7, 2020 · 9 comments
Labels

Comments

@banacorn
Copy link

banacorn commented Sep 7, 2020

I think there's really nothing worth notifying when formatting the document, especially when the formatting command has to be triggered by user by default. It becomes even more distracting when you enable, say, format on save.

截圖 2020-09-07 下午2 22 29

There should be an option in the settings for disabling these notifications.

@konn
Copy link
Collaborator

konn commented Sep 7, 2020

Well, with some projects with large code base, Formatting only a single file can take much, much time and indicator spins for a while. If the codebase is relatively small, notification is not shown so regularly.
I think the cause of this delay is that HLS is waiting for the AST to be parsed, and compiler took much time to reach there..

My altenative suggestion is to add an option that "fallback text-based formatting when no progress has been made after N secs"; taking more than a few seconds for formatting code is not so desirable, so we need some escaping hatch for it, I think.

@banacorn
Copy link
Author

banacorn commented Sep 7, 2020

reason-vscode only shows the notification when the formatting takes too much time. Perhaps we can have a threshold of some kind.

I wonder how much time it takes to format a Haskell file (of different code base sizes).

@lukel97 lukel97 transferred this issue from haskell/vscode-haskell Sep 7, 2020
@lukel97
Copy link
Collaborator

lukel97 commented Sep 7, 2020

I noticed this as well, formatting doesn't usually seem to be that time intensive of an operation so it would make sense to debounce it first.

@pepeiborra
Copy link
Collaborator

Formatting can block for other reasons though, like consulting a cradle. Debouncing is always good, even better if done on the LSP client side since it is a crosscutting concern across all progress notifications.

@georgefst
Copy link
Collaborator

Worth noting, currently the progress notification only shows for Ormolu/Fourmolu, presumably because the others aren't calling withIndefiniteProgress.

The notification doesn't annoy me per se, but what's odd is that it hangs around for almost a second after formatting appears to have finished, even on a trivial file. This could be because we call Ormolu with the idempotency check, which means that everything is actually formatted twice, but it's longer than I'd expect even accounting for that. Anyway, having a threshold of even a second would make this a non-issue for me.

@jneira
Copy link
Member

jneira commented Sep 8, 2020

I am gonna use withIndefiniteProgress in the hlint plugin, so this would affect it

@lukel97 lukel97 mentioned this issue Sep 9, 2020
7 tasks
pepeiborra pushed a commit that referenced this issue Dec 27, 2020
…bust (#400)

* #381, require shake-0.18.5, which ensures progress cancelation is robust

* Fix a GHC 8.8.2 warning

* Don't allow-newer, do pin hie-bios
@Profpatsch
Copy link

I get the problem that with format-on-save, formatting will block saving the file when hls is working on something in the background (especially when it crashes/hangs, which happens if the file has syntax errors on hls start for example).

Requiring me to manually stop hls and interrupt the formatting before I can save the file.

@michaelpj
Copy link
Collaborator

Possibly if we didn't declare them as cancellable the client would not show a UI element (which I think it might be doing just to show the cancel button). I think it's not really necessary to say they're cancelable.

@michaelpj
Copy link
Collaborator

Should be fixed now, we will only get a progress notification if it takes some time.

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

No branches or pull requests

8 participants