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

Show window message when auto extending import lists #1371

Merged
merged 3 commits into from Feb 16, 2021

Conversation

berberman
Copy link
Collaborator

Imo we'd better notify users that we modified source code without their permission.

@pepeiborra
Copy link
Collaborator

I'm not sure this is a good idea. What does this look like in VSCode? And in other clients?

@berberman
Copy link
Collaborator Author

I tried with vscode and emacs:

vscode

Peek

emacs

Peek

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Feb 16, 2021
@berberman berberman removed the merge me Label to trigger pull request merge label Feb 16, 2021
@berberman
Copy link
Collaborator Author

Perhaps it's a bit verbose to include the full file path?

Peek

Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@berberman berberman added the merge me Label to trigger pull request merge label Feb 16, 2021
@mergify mergify bot merged commit 3909291 into haskell:master Feb 16, 2021
@berberman berberman deleted the extend-notify branch February 16, 2021 10:48
@michaelpj
Copy link
Collaborator

It feels to me like this should be a client feature? The spec has AnnotatedTextEdits that let you specify a human-readable name for the edit that is applied, including a part which should be "rendered prominent in the user interface" (https://microsoft.github.io/language-server-protocol/specification#textEdit). It seems like if we return AnnotatedTextEdits with this information then it should be up to the client to show them to the user.

@michaelpj
Copy link
Collaborator

Looks like we maybe need support for them in the lsp library first, though..

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.

None yet

4 participants