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

TUI / GUI tooltip with content from ALEHover #1556

Merged
merged 23 commits into from
May 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c6a45b1
Guard the ballooneval settings
gagbo May 5, 2018
288c791
Mark main objectives to do to get nice Hover
gagbo May 5, 2018
d8c7a86
Make tweaks to make the tooltip work - See " XXX: comments
gagbo May 5, 2018
a86d494
Guard balloon_show call
gagbo May 5, 2018
d5bbb5e
Use return instead of finish for functions
gagbo May 6, 2018
f4ab2b8
ale#hover#show : Add optional arguments to specify arbtirary position
gagbo May 8, 2018
90c9a64
ale#ballon#Disable : Remove feature guards
gagbo May 8, 2018
daed3e3
ale#balloon : Show 'ALEHover' output on balloon if no diagnostic found
gagbo May 8, 2018
c41e062
ale#hover#HandleLSPResponse : remove the check for cursor position
gagbo May 8, 2018
284ec91
ale#balloon#MessageForPos : Change the return of balloonexpr
gagbo May 8, 2018
0ca9747
ale#hover#Response : Feature guard balloon_show calls
gagbo May 8, 2018
d5ba722
ale#hover : always display 'Hover' information in messages
gagbo May 8, 2018
159a371
Merge remote-tracking branch 'upstream/master' into balloon_ALEHover
gagbo May 8, 2018
4a49a05
Merge remote-tracking branch 'upstream/master' into balloon_ALEHover
gagbo May 11, 2018
f22e7a9
{LSP,TS}Response : use only variables from the Response
gagbo May 11, 2018
577dad2
hover#ShowDetails : fix an issue where not having focus broke balloons
gagbo May 11, 2018
e25b763
{LSP,TS}Response : Remove redundant checks for balloon_show call
gagbo May 11, 2018
d8ac705
balloonexpr? : Add a flag to separate hover#Show() calls
gagbo May 11, 2018
e826b8b
ale#hover#hover_map : Protect accesses to hover_map
gagbo May 11, 2018
7156ad4
Raise timeout to try to get Appveyor happy
gagbo May 11, 2018
1cbc3d4
Merge remote-tracking branch 'upstream/master' into balloon_ALEHover
gagbo May 16, 2018
32c50d4
Review : Fix comments
gagbo May 16, 2018
273ed54
Review : pass the optional argument 'called_from_balloonexpr' in a Dict
gagbo May 16, 2018
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
25 changes: 22 additions & 3 deletions autoload/ale/balloon.vim
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,36 @@ function! ale#balloon#MessageForPos(bufnr, lnum, col) abort
let l:loclist = get(g:ale_buffer_info, a:bufnr, {'loclist': []}).loclist
let l:index = ale#util#BinarySearch(l:loclist, a:bufnr, a:lnum, a:col)

return l:index >= 0 ? l:loclist[l:index].text : ''
" Show the diagnostics message if found, 'Hover' output otherwise
if l:index >= 0
return l:loclist[l:index].text
else
call ale#hover#Show(a:bufnr, a:lnum, a:col, {'called_from_balloonexpr': 1})
return ''
endif
endfunction

function! ale#balloon#Expr() abort
return ale#balloon#MessageForPos(v:beval_bufnr, v:beval_lnum, v:beval_col)
endfunction

function! ale#balloon#Disable() abort
set noballooneval balloonexpr=
set noballooneval noballoonevalterm
set balloonexpr=
endfunction

function! ale#balloon#Enable() abort
set ballooneval balloonexpr=ale#balloon#Expr()
if !has('balloon_eval') && !has('balloon_eval_term')
return
endif

if has('balloon_eval')
set ballooneval
endif

if has('balloon_eval_term')
set balloonevalterm
endif

set balloonexpr=ale#balloon#Expr()
endfunction
76 changes: 50 additions & 26 deletions autoload/ale/hover.vim
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ function! ale#hover#HandleTSServerResponse(conn_id, response) abort

if get(a:response, 'success', v:false) is v:true
\&& get(a:response, 'body', v:null) isnot v:null
call ale#util#ShowMessage(a:response.body.displayString)
if get(l:options, 'hover_from_balloonexpr', 0)
\&& exists('*balloon_show')
\&& ale#Var(l:options.buffer, 'set_balloons')
Copy link
Member

Choose a reason for hiding this comment

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

We'll need to think about the meaning of the set_balloons setting. I guess it makes sense how you've done it here. I doubt many people really look at the balloons for the error messages at the moment.

I say we should do it how you've done it here, and update the documentation to say that the option also enables hover information for LSP and tsserver.

I might update the tsserver stuff myself, or you can give it a go if you want. If you install TypeScript into node_modules for a project, it should be pretty easy to get it running tsserver comes with TypeScript.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed that the ale#hover callbacks were only called by ale#hover#Show, and therefore the value of set_ballons in this context is only relevant for the Hover information to begin with. I think this specific part of the code is not related to the actual design decision, it's really just a sanity check on what the user asked for relatively to balloons before we make it appear.

The part that makes the meaning of set_balloons matter in a "hover vs diagnostics" way is in ale#balloon#MessageForPos (where I decided that if diagnostics are found, show them and don't even try to get hover information for the position).

call balloon_show(a:response.body.displayString)
else
call ale#util#ShowMessage(a:response.body.displayString)
endif
endif
endif
endfunction
Expand All @@ -34,15 +40,18 @@ function! ale#hover#HandleLSPResponse(conn_id, response) abort
\&& has_key(s:hover_map, a:response.id)
let l:options = remove(s:hover_map, a:response.id)

let l:buffer = bufnr('')
let [l:line, l:column] = getcurpos()[1:2]
let l:end = len(getline(l:line))

if l:buffer isnot l:options.buffer
\|| l:line isnot l:options.line
\|| min([l:column, l:end]) isnot min([l:options.column, l:end])
" Cancel display the message if the cursor has moved.
return
" If the call did __not__ come from balloonexpr...
if !get(l:options, 'hover_from_balloonexpr', 0)
let l:buffer = bufnr('')
let [l:line, l:column] = getcurpos()[1:2]
let l:end = len(getline(l:line))

if l:buffer isnot l:options.buffer
\|| l:line isnot l:options.line
\|| min([l:column, l:end]) isnot min([l:options.column, l:end])
" ... Cancel display the message if the cursor has moved.
return
endif
endif

" The result can be a Dictionary item, a List of the same, or null.
Expand Down Expand Up @@ -71,21 +80,24 @@ function! ale#hover#HandleLSPResponse(conn_id, response) abort
let l:str = substitute(l:str, '^\s*\(.\{-}\)\s*$', '\1', '')

if !empty(l:str)
call ale#util#ShowMessage(l:str)
if get(l:options, 'hover_from_balloonexpr', 0)
\&& exists('*balloon_show')
\&& ale#Var(l:options.buffer, 'set_balloons')
call balloon_show(l:str)
else
call ale#util#ShowMessage(l:str)
endif
endif
endif
endif
endfunction

function! s:ShowDetails(linter) abort
let l:buffer = bufnr('')
let [l:line, l:column] = getcurpos()[1:2]

function! s:ShowDetails(linter, buffer, line, column, opt) abort
let l:Callback = a:linter.lsp is# 'tsserver'
\ ? function('ale#hover#HandleTSServerResponse')
\ : function('ale#hover#HandleLSPResponse')

let l:lsp_details = ale#linter#StartLSP(l:buffer, a:linter, l:Callback)
let l:lsp_details = ale#linter#StartLSP(a:buffer, a:linter, l:Callback)

if empty(l:lsp_details)
return 0
Expand All @@ -95,34 +107,46 @@ function! s:ShowDetails(linter) abort
let l:root = l:lsp_details.project_root

if a:linter.lsp is# 'tsserver'
let l:column = a:column

let l:message = ale#lsp#tsserver_message#Quickinfo(
\ l:buffer,
\ l:line,
\ a:buffer,
\ a:line,
\ l:column
\)
else
" Send a message saying the buffer has changed first, or the
" hover position probably won't make sense.
call ale#lsp#Send(l:id, ale#lsp#message#DidChange(l:buffer), l:root)
call ale#lsp#Send(l:id, ale#lsp#message#DidChange(a:buffer), l:root)

let l:column = min([l:column, len(getline(l:line))])
let l:column = min([a:column, len(getbufline(a:buffer, a:line)[0])])

let l:message = ale#lsp#message#Hover(l:buffer, l:line, l:column)
let l:message = ale#lsp#message#Hover(a:buffer, a:line, l:column)
endif

let l:request_id = ale#lsp#Send(l:id, l:message, l:root)

let s:hover_map[l:request_id] = {
\ 'buffer': l:buffer,
\ 'line': l:line,
\ 'buffer': a:buffer,
\ 'line': a:line,
\ 'column': l:column,
\ 'hover_from_balloonexpr': get(a:opt, 'called_from_balloonexpr', 0),
\}
endfunction

function! ale#hover#Show() abort
for l:linter in ale#linter#Get(&filetype)
" Obtain Hover information for the specified position
" Pass optional arguments in the dictionary opt.
" Currently, only one key/value is useful:
" - called_from_balloonexpr, this flag marks if we want the result from this
" ale#hover#Show to display in a balloon if possible
"
" Currently, the callbacks displays the info from hover :
" - in the balloon if opt.called_from_balloonexpr and balloon_show is detected
" - as status message otherwise
function! ale#hover#Show(buffer, line, col, opt) abort
for l:linter in ale#linter#Get(getbufvar(a:buffer, '&filetype'))
if !empty(l:linter.lsp)
call s:ShowDetails(l:linter)
call s:ShowDetails(l:linter, a:buffer, a:line, a:col, a:opt)
endif
endfor
endfunction
3 changes: 2 additions & 1 deletion plugin/ale.vim
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,8 @@ command! -bar ALEGoToDefinitionInTab :call ale#definition#GoTo({'open_in_tab': 1
command! -bar ALEFindReferences :call ale#references#Find()

" Get information for the cursor.
command! -bar ALEHover :call ale#hover#Show()
command! -bar ALEHover :call ale#hover#Show(bufnr(''), getcurpos()[1],
\ getcurpos()[2], {})

" <Plug> mappings for commands
nnoremap <silent> <Plug>(ale_previous) :ALEPrevious<Return>
Expand Down
2 changes: 1 addition & 1 deletion test/smoke_test.vader
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Execute(Linters should run in PowerShell too):
\})

call ale#Lint()
call ale#engine#WaitForJobs(2000)
call ale#engine#WaitForJobs(4000)

AssertEqual [
\ {
Expand Down