This repository has been archived by the owner on Jan 3, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 156
Close previously open code action windows #368
Open
dodomorandi
wants to merge
2
commits into
simrat39:master
Choose a base branch
from
dodomorandi:dont-open-multiple-code-actions
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the steps from #362, now the second code action picker doesn't open at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean that this is expected for you or you wanted a different behavior?
I tried to avoid multiple code actions opened, which has the consequence that when the LSP is still loading the stuck before trying to open the floats. Which leads to undetermined order of resolution for the handling of the cleanup state. For instance, during my tests I saw no code action windows opening, which to me is still better than the previous behavior.
Let me know if this approach is ok or if you would rather opt for something different☺️ .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should follow the native neovim behavior of
vim.lsp.buf.code_action
?🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think I got your point: you should be able to close the previous code action instead of avoiding the opening of a new one, right?
I did not thing about this kind of situation because if you run
vim.lsp.buf.code_action
you natively don't get any window, you just get the input message to choose the code action, you cannot open a new action code without leaving (and making disappear) the current choice.In any case, I simply did not realize that if you just focus out of the floating window (instead of quitting it), it will just be left there and if you try to open a new code action both are closed. But also think that the problem is I mainly related to your second point, not the third one. In other words, I think that it should not be possible to trigger a second code action without the first one already been closed.
Do we agree? As I said, I was not really understanding because "focusing out" was just "quitting" the floating window, now I also think that the issue is still there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MunifTanjim I added two events listener in order to detect when the code action windows are left, in order to close them. Interestingly enough, I started noticing a different issue if you try to repeatedly open a code action when the LSP is still not ready, maybe related to the fact that
state.actions
is cleared on cleanup (which I run in case when leaving the code action windows) but there are concurrent handling of pending code actions.Let me know what you think, for both the change and the new issue. Said that, I noticed what you are doing for this project and I really appreciate it. So, thank you for your efforts.