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.

Sign up for GitHub

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

Improve handling of key mapping #325

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+36 −2
Open

Improve handling of key mapping #325

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
876fe59
Remove redundant call to mapKeys()
gpanders Aug 10, 2020
a5a8cfc
Only map keys when server is running
gpanders Aug 10, 2020
6f05784
Unmap keys when server shuts down
gpanders Aug 10, 2020
c8248dc
Save mapped keys to buffer-local variable
gpanders Oct 5, 2020
6d437f4
Only unmap keys if server is used by current buffer
gpanders Oct 5, 2020
19e4e6c
Check server status and unmap keys if not running
gpanders Oct 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 48 additions & 13 deletions autoload/lsc/config.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,8 @@ function! s:ApplyDefaults(config) abort
return l:merged
endfunction

function! lsc#config#mapKeys() abort
if !exists('g:lsc_auto_map')
\ || (type(g:lsc_auto_map) == type(v:true) && !g:lsc_auto_map)
\ || (type(g:lsc_auto_map) == type(0) && !g:lsc_auto_map)
return
endif
let l:maps = s:ApplyDefaults(g:lsc_auto_map)
if type(l:maps) != type({})
call lsc#message#error('g:lsc_auto_map must be a bool or dict')
return
endif

for command in [
function! s:CommandList() abort
return [
\ 'GoToDefinition',
\ 'GoToDefinitionSplit',
\ 'FindReferences',
Expand All @@ -64,6 +53,22 @@ function! lsc#config#mapKeys() abort
\ 'WorkspaceSymbol',
\ 'SignatureHelp',
\] + (get(g:, 'lsc_enable_apply_edit', 1) ? ['Rename'] : [])
endfunction

function! lsc#config#mapKeys() abort
if !exists('g:lsc_auto_map')
\ || (type(g:lsc_auto_map) == type(v:true) && !g:lsc_auto_map)
\ || (type(g:lsc_auto_map) == type(0) && !g:lsc_auto_map)
return
endif
let l:maps = s:ApplyDefaults(g:lsc_auto_map)
if type(l:maps) != type({})
call lsc#message#error('g:lsc_auto_map must be a bool or dict')
return
endif

let b:lsc_save = {}
for command in s:CommandList()
let lhs = get(l:maps, command, [])
if type(lhs) != type('') && type(lhs) != type([])
continue
Expand All @@ -75,18 +80,48 @@ function! lsc#config#mapKeys() abort
if has_key(l:maps, 'Completion') &&
\ type(l:maps['Completion']) == type('') &&
\ len(l:maps['Completion']) > 0
let b:lsc_save[l:maps['Completion']] = getbufvar('', l:maps['Completion'])
execute 'setlocal '.l:maps['Completion'].'=lsc#complete#complete'
endif
if has_key(l:maps, 'ShowHover')
let l:show_hover = l:maps['ShowHover']
if type(l:show_hover) == type(v:true) || type(l:show_hover) == type(0)
if l:show_hover
let b:lsc_save.keywordprg = &l:keywordprg
setlocal keywordprg=:LSClientShowHover
endif
endif
endif
endfunction

function! lsc#config#unmapKeys() abort
if exists('b:lsc_save')
for opt in keys(b:lsc_save)
execute 'setlocal '.opt.'='.b:lsc_save[opt]
endfor
unlet b:lsc_save
endif

if !exists('g:lsc_auto_map')
\ || (type(g:lsc_auto_map) == type(v:true) && !g:lsc_auto_map)
\ || (type(g:lsc_auto_map) == type(0) && !g:lsc_auto_map)
return
endif
let l:maps = s:ApplyDefaults(g:lsc_auto_map)

for command in s:CommandList()
let lhs = get(l:maps, command, [])
if type(lhs) != type('') && type(lhs) != type([])
continue
endif
for m in type(lhs) == type([]) ? lhs : [lhs]
if get(maparg(m, 'n', v:false, v:true), 'buffer')
execute 'nunmap <buffer>'.m
endif
endfor
endfor
endfunction

" Wraps [Callback] with a function that will first translate a result through a
" user provided translation.
function! lsc#config#responseHook(server, method, Callback) abort
Expand Down
1 change: 0 additions & 1 deletion autoload/lsc/file.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ endfunction
" Run language servers for this filetype if they aren't already running and
" flush file changes.
function! lsc#file#onOpen() abort
call lsc#config#mapKeys()
if &modifiable && expand('%') !~# '\vfugitive:///'
call lsc#server#start(&filetype)
call s:FlushChanges(lsc#file#fullPath(), &filetype)
Expand Down
3 changes: 3 additions & 0 deletions autoload/lsc/server.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ function! s:Kill(server, status, OnExit) abort
call a:server._channel.notify('exit', v:null)
endif
if a:OnExit != v:null | call a:OnExit() | endif
call lsc#config#unmapKeys()
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like this will unmap keys for whichever buffer is current when the server exists - there isn't a guarantee that the current buffer is one handled by the server which exited, and we'll missed unmapping keys on background buffers as well.

I think we'll need to loop over buffers for each filetype and unmap in each of them which makes things a little trickier.

Copy link
Contributor Author

@gpanders gpanders Oct 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You're right, this is tricky because as far as I know Vim doesn't provide a method for (un)mapping buffer-local keys for buffers other than the current one. Which means to unmap the buffer-local keys in all buffers associated with the language server, we would either need to:

  1. Iterate through each buffer and unmap the keys (probably slow and requires switching buffers which has all other kinds of pitfalls)

  2. Hook into the BufEnter autocmd and check the status of the server associated with that buffer's filetype: if the status is disabled then unmap the keys. This essentially unmaps "on demand".

Unless there is something I'm overlooking then it seems to me that option 2 is the clear winner. I will update the PR with these changes for your review.

endfunction
return a:server.request('shutdown', v:null, funcref('Exit'))
endfunction
Expand Down Expand Up @@ -119,6 +120,7 @@ endfunction
function! s:Start(server) abort
if has_key(a:server, '_channel')
" Server is already running
call lsc#config#mapKeys()
return
endif
let l:command = a:server.config.command
Expand All @@ -145,6 +147,7 @@ function! s:Start(server) abort
for filetype in a:server.filetypes
call lsc#file#trackAll(filetype)
endfor
call lsc#config#mapKeys()
endfunction
if exists('g:lsc_trace_level') &&
\ index(['off', 'messages', 'verbose'], g:lsc_trace_level) >= 0
Expand Down
1 change: 0 additions & 1 deletion plugin/lsc.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ endfunction

function! s:OnOpen() abort
if !has_key(g:lsc_servers_by_filetype, &filetype) | return | endif
call lsc#config#mapKeys()
if !lsc#server#filetypeActive(&filetype) | return | endif
call lsc#file#onOpen()
endfunction
Expand Down