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

Customizable TH warning #2781

Merged
merged 4 commits into from
Mar 19, 2022
Merged

Customizable TH warning #2781

merged 4 commits into from
Mar 19, 2022

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Mar 14, 2022

I want to change the LspT action that gets run the first time that we run into unsupported Template Haskell. This can also be used to completely disable the TH warning (programmatically) .

@michaelpj
Copy link
Collaborator

Hmm, given the existence of the LSP showMessage recorder, we can almost replace this with just logging. But I think it's currently only showing logs at error severity, not warnings. It would be nicer if this was "just" a log, though.

@pepeiborra
Copy link
Collaborator Author

Hmm, given the existence of the LSP showMessage recorder, we can almost replace this with just logging. But I think it's currently only showing logs at error severity, not warnings. It would be nicer if this was "just" a log, though.

Agreed

@michaelpj
Copy link
Collaborator

Would it be crazy to show warnings to the user as well? Or is that going to be too aggressive in general?

@pepeiborra
Copy link
Collaborator Author

Would it be crazy to show warnings to the user as well? Or is that going to be too aggressive in general?

Do we have any warning logs?

@michaelpj
Copy link
Collaborator

Yes, quite a few.

The flip side of this is: how bad would it be if this message went only to the log? Note that now we will send it to the client log with window/logMessage, so it should be readily visible if anyone looks for diagnostics. Do we need it to be a "push" notification?

@pepeiborra
Copy link
Collaborator Author

Yes, quite a few.

The flip side of this is: how bad would it be if this message went only to the log? Note that now we will send it to the client log with window/logMessage, so it should be readily visible if anyone looks for diagnostics. Do we need it to be a "push" notification?

Sending the message to the log stream will provide useful diagnostics, but the goal of this message is to create awareness. I suspect that most HLS users, specially new ones, will not even bother to check the logs in the event of a crash.

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Mar 17, 2022
@mergify mergify bot merged commit 252c365 into master Mar 19, 2022
July541 pushed a commit to July541/haskell-language-server that referenced this pull request Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants