[vim/vim] Proposal: Add end_lnum and end_col to quickfix entry (#8393)
thinca
Modern tools may be able to specify a range when outputting the location of the source code.
- LSP reports the location of errors etc. in the range.
- The Rust compiler can report error locations in a range.
- ripgrep can get information on the end position of the match in JSON format.
Quickfix is a common part of Vim, where many plugins output location information to here, and many plugins get location information from here.
The fact that the quickfix entry has range information creates orthogonality between plugins.
For example, we can create a plugin that properly highlights the range specified by quickfix.
For example, even if there are multiple errors on the same line, we can pop up the appropriate error message for the cursor position.
If quickfix doesn't have range information, we can only collect location information and highlight it in one plugin. This is because there is no way to pass information between plugins.
This pull request does not contain any changes regarding errorformat. I think it will be necessary eventually, but first I made a pull request at this stage to see if this proposal would be accepted.
What do you think about this?
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/8393
Commit Summary
- Add end_lnum and end_col to quickfix
File Changes
- M runtime/doc/eval.txt (3)
- M src/quickfix.c (71)
- M src/testdir/dumps/Test_quickfix_cwindow_1.dump (6)
- M src/testdir/dumps/Test_quickfix_cwindow_2.dump (6)
- M src/testdir/test_quickfix.vim (2)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
codecov[bot]
Codecov Report
Merging #8393 (2f777c5) into master (41a7f82) will decrease coverage by
87.34%.
The diff coverage is0.00%.
@@ Coverage Diff @@ ## master #8393 +/- ## =========================================== - Coverage 89.85% 2.50% -87.35% =========================================== Files 149 147 -2 Lines 167465 162318 -5147 =========================================== - Hits 150469 4068 -146401 - Misses 16996 158250 +141254
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | ? |
|
| huge-gcc-none | ? |
|
| huge-gcc-testgui | ? |
|
| huge-gcc-unittests | 2.50% <0.00%> (-0.01%) |
⬇️ |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/quickfix.c | 0.00% <0.00%> (-94.87%) |
⬇️ |
| src/float.c | 0.00% <0.00%> (-98.91%) |
⬇️ |
| src/sha256.c | 0.00% <0.00%> (-97.96%) |
⬇️ |
| src/digraph.c | 0.00% <0.00%> (-97.78%) |
⬇️ |
| src/gui_gtk_f.c | 0.00% <0.00%> (-97.54%) |
⬇️ |
| src/match.c | 0.00% <0.00%> (-97.13%) |
⬇️ |
| src/crypt_zip.c | 0.00% <0.00%> (-97.06%) |
⬇️ |
| src/sound.c | 0.00% <0.00%> (-97.00%) |
⬇️ |
| src/evalbuffer.c | 0.00% <0.00%> (-96.83%) |
⬇️ |
| src/cmdhist.c | 0.00% <0.00%> (-96.63%) |
⬇️ |
| ... and 136 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update 41a7f82...2f777c5. Read the comment docs.
Bram Moolenaar
Thanks, this can be useful.
I'll let @yegappan make comments, he has been doing work on quickfix code.
Yegappan Lakshmanan
Thanks, this can be useful.
I'll let @yegappan make comments, he has been doing work on quickfix code.
vim-dev ML
On Wed, Jun 16, 2021 at 12:37 PM Bram Moolenaar ***@***.***>
wrote:
> Thanks, this can be useful.
> been doing work on quickfix code.
>
>
>
The changes look good to me. Can you add a few test cases for setqflist(),
getqflist(), :vimgrep, :helpgrep, :clist and :cwindow with the end line
number and column number?
Regards,
Yegappan
thinca
I added some patterns to existing test cases. Is this enough?
I tweaked representation of range in quickfix window/:clist.
lnum |
end_lnum |
col |
end_col |
representation |
|---|---|---|---|---|
| yes | no | no | no | 10 |
| yes | no | yes | no | 10 col 5 |
| yes | no | yes | yes | 10 col 5-8 |
| yes | yes | no | no | 10-12 |
| yes | yes | yes | no | 10-12 col 5 |
| yes | yes | yes | yes | 10-12 col 5-8 |
—
You are receiving this because you commented.
Yegappan Lakshmanan
I added some patterns to existing test cases. Is this enough?
vim-dev ML
On Thu, Jun 17, 2021 at 9:39 AM thinca ***@***.***> wrote:
> I added some patterns to existing test cases. Is this enough?
>
Yes. This looks good. Thanks for the patch and the tests.
Regards,
Yegappan
> I tweaked representation of range in quickfix window/:clist.
> lnum end_lnum col end_col representation
> yes no no no 10
> yes no yes no 10 col 5
> yes no yes yes 10 col 5-8
> yes yes no no 10-12
> yes yes yes no 10-12 col 5
> yes yes yes yes 10-12 col 5-8
>
>
>
Shane-XB-Qian
@Shane-XB-Qian commented on this pull request.
In src/testdir/test_quickfix.vim:
> @@ -5281,7 +5308,7 @@ func Test_add_invalid_entry_with_qf_window()
call setqflist(['bb'], 'a')
call assert_equal(1, line('$'))
call assert_equal(['Xfile1|10| aa'], getline(1, '$'))
- call assert_equal([{'lnum': 10, 'bufnr': bufnr('Xfile1'), 'col': 0, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'module': '', 'text': 'aa'}], getqflist())
+ call assert_equal([{'lnum': 10, 'end_lnum': 0, 'bufnr': bufnr('Xfile1'), 'col': 0, 'end_col': 0, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'module': '', 'text': 'aa'}], getqflist())
what if 'end_lnum' and/or 'end_col' is '0' (or even was a negative value), then what's the getline(1, '$') ?
would it be still compatible like Xfile1|10| aa vs not Xfile1|10-0| aa ? the later case should be not wanted.
—
You are receiving this because you commented.
thinca
@thinca commented on this pull request.
In src/testdir/test_quickfix.vim:
> @@ -5281,7 +5308,7 @@ func Test_add_invalid_entry_with_qf_window()
call setqflist(['bb'], 'a')
call assert_equal(1, line('$'))
call assert_equal(['Xfile1|10| aa'], getline(1, '$'))
- call assert_equal([{'lnum': 10, 'bufnr': bufnr('Xfile1'), 'col': 0, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'module': '', 'text': 'aa'}], getqflist())
+ call assert_equal([{'lnum': 10, 'end_lnum': 0, 'bufnr': bufnr('Xfile1'), 'col': 0, 'end_col': 0, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'module': '', 'text': 'aa'}], getqflist())
Don't worry.
end_lnum and end_col are just optional values. The text in the quickfix window is never changed when they are zero. It will be displayed as before. See the comment above.
—
You are receiving this because you commented.
Shane-XB-Qian
@Shane-XB-Qian commented on this pull request.
In src/testdir/test_quickfix.vim:
> @@ -5281,7 +5308,7 @@ func Test_add_invalid_entry_with_qf_window()
call setqflist(['bb'], 'a')
call assert_equal(1, line('$'))
call assert_equal(['Xfile1|10| aa'], getline(1, '$'))
- call assert_equal([{'lnum': 10, 'bufnr': bufnr('Xfile1'), 'col': 0, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'module': '', 'text': 'aa'}], getqflist())
+ call assert_equal([{'lnum': 10, 'end_lnum': 0, 'bufnr': bufnr('Xfile1'), 'col': 0, 'end_col': 0, 'pattern': '', 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'module': '', 'text': 'aa'}], getqflist())
ok, wish so.
i mean let it be part of that test too. :-)
—
You are receiving this because you commented.