add preview for ALEFindReferences#5001
Conversation
bretello
force-pushed
the
add-preview-alefindreferences
branch
from
2d0a51c to
8f11e59
Compare
bretello
force-pushed
the
add-preview-alefindreferences
branch
from
8f11e59 to
f13c313
Compare
autoload/ale/references.vim
Outdated
| try | ||
| let l:line_text = readfile(l:filename)[l:line] | ||
| catch | ||
| " cannot read file, this happens in tests |
There was a problem hiding this comment.
This is a workaround to the following test failures (which use a dummy filename)
LSP reference responses should be put to quickfix and LSP reference responses should be handled
I'll try and come up with a better solution for tests.
|
I guess this is kind of a duplicate of #4982 |
w0rp
left a comment
w0rp
left a comment
There was a problem hiding this comment.
This is a nice addition! Have a look at my comments here.
autoload/ale/references.vim
Outdated
| \ 'column': l:col + 1, | ||
| \} | ||
|
|
||
| if exists('l:line_text') |
There was a problem hiding this comment.
Instead of using an exists() check let's default the variable to an empty string. Can we avoid the conditional value and always return the Dictionary directly with text above or match here set to an empty string?
There was a problem hiding this comment.
Cleaned this section up a bit to address this and to keep it similar to ale#references#FormatTSResponseItem
autoload/ale/references.vim
Outdated
| let l:filename = ale#util#ToResource(a:response_item.uri) | ||
|
|
||
| try | ||
| let l:line_text = readfile(l:filename)[l:line] |
There was a problem hiding this comment.
Let's add an option to disable showing the contents of files for references. We can enable showing the contents of files by default. Lets make it a minus option so you can write commands to explicitly enable or disable line previews. Have a look at how ale#references#Find lets you specify where to output references and then falls back on ale_default_navigation.
If we are displaying preview text do we need to consider the character start and line and character end of the range? Just something worth considering and seeing how it looks.
There was a problem hiding this comment.
Thanks for the review @w0rp .
I included the first suggestion in the last push (a012900)
Regarding the second suggestion, I initially implemented it showing the match over the range, but it ended up only showing the reference being searched, which wasn't very useful.
I also added a few tests and updated the docs.
bretello
force-pushed
the
add-preview-alefindreferences
branch
4 times, most recently
from
32798f9 to
7cff51e
Compare
…ntents option to configure default behaviour
bretello
force-pushed
the
add-preview-alefindreferences
branch
from
7cff51e to
d90648e
Compare
|
Cheers! 🍻 |
|
|
||
| if get(a:options, 'show_contents') == 1 | ||
| try | ||
| let l:line_text = substitute(readfile(l:filename)[l:line], '^\s*\(.\{-}\)\s*$', '\1', '') |
There was a problem hiding this comment.
I've merged this, and thanks for this. An improvement we could try here is passing the max argument to readfile to read only up to the line we need.
* add preview for ALEFindReferences * ALEFindReferences: add -contents options and g:ale_references_show_contents option to configure default behaviour * tests: add tests for ALEFindReferences -contents feature * tests: update tsserver references tests with show_contents args
2 participants
This PR adds support line preview in
ALEFindReferencesoutputasciinema