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

Treat window/progress messages as log messages #316

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gpanders
Copy link
Contributor

@gpanders gpanders commented Jul 7, 2020

These messages should only be displayed if log_level is set to 'Log'.

Back in #130 support was added for window/progress messages which are sent by rls. However, these messages are a bit incessant and quickly get very annoying, and as it is now there is no way to disable them. This PR treats these messages as "Log" level messages and will therefore only display them if log_level is set to 'Log'.

I don't love the hard coded 4 in here and I am open to suggestions on a better solution, but as far as I can tell the log levels aren't enumerated anywhere so hard coding is the only possible approach (the window/progress messages do not have a type parameter like the window/logMessage messages do). EDIT: I added a new function lsc#config#messageType so that we can avoid using magic numbers.

@natebosch
Copy link
Owner

Thanks for the PR.

I'm a little torn on the best way to handle this since I don't have a good understanding of how most servers and users use progress. One thing to note here is that the window/progress request supported here is not official - it was added speculatively and looked like it may have been adopted into the protocol, but then the protocol went with $/progress instead and has different semantics and interfaces.

I'm not sure that 'Log' would be the level most users expect a progress message would correlate to, but I'm also not eager to add even more config around this feature which is not part of the official spec.

I think the optimal solution here would be to drop window/progress support entirely in favor of $/progress support, but I'm not sure how much effort that would take or when I might be able to do that. We could also look at dropping the requirement that server specific notifications start with $ and shift the support to the server config.

elseif a:method =~? '\v^\$'
call lsc#config#handleNotification(a:server, a:method, a:params)
endif

Would you be happy instead if these messages were just suppressed always, and users who are interested in them would set up their own handling?

@gpanders
Copy link
Contributor Author

Would you be happy instead if these messages were just suppressed always, and users who are interested in them would set up their own handling?

That is a perfectly acceptable solution as far as I'm concerned! Thank you for the elaborate response.

These messages should only be displayed if `log_level` is set to
`'Log'`.
This converts a message type by name into a log level value
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 this pull request may close these issues.

2 participants