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

Fix the problem of key bindings in finder buffers #4122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Shuangcheng-Ni
Copy link

@Shuangcheng-Ni Shuangcheng-Ni commented Jan 30, 2023

PR Prelude

Thank you for working on YCM! :)

Please complete these steps and check these boxes (by putting an x inside
the brackets) before filing your PR:

  • I have read and understood YCM's CONTRIBUTING document.
  • I have read and understood YCM's CODE_OF_CONDUCT document.
  • I have included tests for the changes in my PR. If not, I have included a
    rationale for why I haven't.
  • I understand my PR may be closed if it becomes obvious I didn't
    actually perform all of these steps.

Why this change is necessary and useful

As we discussed on gitter, some extra insert-mode key bindings might dismiss the finder buffer. For example, if a user has something like inoremap <BS> <C-r>=auto_pair()<CR> in his/her .vimrc, the finder buffer would immediately disappear after pressing <BS>.
This commit fixes the problem. It tries to override all the global mappings with some buffer-local mappings that remap each previously mapped key sequence to itself in the finder buffer, while the previously defined buffer-local mappings in the finder buffer are not overridden.

[Please explain in detail why the changes in this PR are needed.]


This change is Reviewable

Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

Thanks for making a PR, but I'm not sure this is the way to go. I'm not sure that we really want to remove user mappings.

Is the issue you experienced due to the <C-r>= part ? I.e. CmdWinEnter could be used to disable YCM closing the popup. Or perhaps there's a way to detect if we're halfway a mapping when the bufleave event happens.

This seems more likely to cause problems than solve them IMO.

In any case, we'd need extensive test for this change if we were to merge it (as it's complex, and fiddly).

Reviewable status: 0 of 2 LGTMs obtained

@Shuangcheng-Ni
Copy link
Author

The new mappings defined in my additional code are buffer-local mappings and only works in the finder buffer. They do not affect other buffers. They are just used to override the global mappings in the finder buffer and make each key sequence take its original form.
Most insert-mode mappings take the form of imap sth. <C-o>:command<CR>, imap sth. <Cmd>command<CR> or imap sth. <C-r>=function()<CR>. All these mappings have the side effect of closing the finder buffer. I think there is no need to preserve them.
As you said, it is also a good idea to disable some of the autocmds in augroup YCMPromptFindSymbol. But users will have to quit the prompt buffer by commands like :q then.

augroup YCMPromptFindSymbol
  autocmd!
  autocmd TextChanged,TextChangedI <buffer> call s:OnQueryTextChanged()
  autocmd WinLeave <buffer> call s:Cancel()
  autocmd CmdLineEnter <buffer> call s:Cancel()
augroup END

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.

None yet

2 participants