yegappan / mru

Most Recently Used (MRU) Vim Plugin
Other
259 stars 48 forks source link

Bugfix - <silent> key mappings prevent Vim from asking the user to enter the encryption key #56

Closed jamescherti closed 2 years ago

jamescherti commented 2 years ago

MRU/edit/view keyboard mappings: remove <silent> and replace :call with <cmd>call.

Reason: <silent> prevents Vim from asking the user "Enter encryption key:" when an encrypted file is opened by MRU (:help encryption).

yegappan commented 2 years ago

The <cmd> feature is available only in recent versions of Vim. The MRU plugin supports older Vim versions also. So this change cannot be made to the plugin.

jamescherti commented 2 years ago

This pull request fixes a bug (MRU key mappings prevent Vim from asking "Enter encryption key:" when an encrypted file is opened by MRU).

  1. I could improve this pull request by making MRU check if <cmd> is supported by Vim.
  2. Another solution is to remove <silent> only.

What do you prefer/suggest?

yegappan commented 2 years ago

Can you add a Vim version check for the <cmd> support? If it is available, then <cmd> can be used in the maps, otherwise the existing method for the maps can be used.

jamescherti commented 2 years ago

There is probably a way to check if <cmd> is supported by Vim. I will do some research and come back to you, @yegappan .

yegappan commented 2 years ago

The support for <cmd> was added by patch 8.2.1978 (https://github.com/vim/vim/commit/957cf67d50516ba98716f59c9e1cb6412ec1535d). You can check for this patch using has('patch-8.2.1978').

jamescherti commented 2 years ago

Yes, @yegappan . I will use has("patch-8.2.1978") to check if Vim supports <cmd>.

jamescherti commented 2 years ago

I updated the pull request with the suggested changes.

MRU key mappings will use <cmd> when the function has('patch-8.2.1978') returns a non-zero value. Otherwise, the key mappings will use <silent>.

yegappan commented 2 years ago

Test16 (which opens multiple files from the MRU window in normal and visual mode) is failing with this change.

jamescherti commented 2 years ago

Test16 seems to fail because of the following two key mappings and only in versions of Vim that support <cmd>:

  execute 'nnoremap <buffer> ' . l:silent . '<CR> ' .
        \ l:cmd . 'call <SID>MRU_Select_File_Cmd("edit,useopen")<CR>'
  execute 'vnoremap <buffer> ' . l:silent . '<CR> ' .
        \ l:cmd . 'call <SID>MRU_Select_File_Cmd("edit,useopen")<CR>'

I will investigate and get back to you.

jamescherti commented 2 years ago

I have fixed the issue in the latest commit. Now all unit tests pass.

codecov-commenter commented 2 years ago

Codecov Report

Merging #56 (cafcfa4) into master (bcaecf1) will decrease coverage by 1.33%. The diff coverage is 77.27%.

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
- Coverage   82.74%   81.40%   -1.34%     
==========================================
  Files           1        1              
  Lines         539      554      +15     
==========================================
+ Hits          446      451       +5     
- Misses         93      103      +10     
Impacted Files Coverage Δ
plugin/mru.vim 81.40% <77.27%> (-1.34%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

jamescherti commented 2 years ago

Thank you for merging this pull request, @yegappan.