patch 8.0.1669: :vimgrep may add entries to the wrong quickfix list
Problem: :vimgrep may add entries to the wrong quickfix list.
Solution: Use the list identifier. (Yegappan Lakshmanan)
diff --git a/src/quickfix.c b/src/quickfix.c
index f32dcff..e5bd2d0 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -4160,6 +4160,21 @@
}
/*
+ * Return the quickfix/location list number with the given identifier.
+ * Returns -1 if list is not found.
+ */
+ static int
+qf_id2nr(qf_info_T *qi, int_u qfid)
+{
+ int qf_idx;
+
+ for (qf_idx = 0; qf_idx < qi->qf_listcount; qf_idx++)
+ if (qi->qf_lists[qf_idx].qf_id == qfid)
+ return qf_idx;
+ return -1;
+}
+
+/*
* Return the vimgrep autocmd name.
*/
static char_u *
@@ -4272,40 +4287,32 @@
*/
static int
vgr_qflist_valid(
+ win_T *wp,
qf_info_T *qi,
- int_u save_qfid,
- qfline_T *cur_qf_start,
- int loclist_cmd,
+ int_u qfid,
char_u *title)
{
- if (loclist_cmd)
+ /* Verify that the quickfix/location list was not freed by an autocmd */
+ if (!qflist_valid(wp, qfid))
{
- /*
- * Verify that the location list is still valid. An autocmd might
- * have freed the location list.
- */
- if (!qflist_valid(curwin, save_qfid))
+ if (wp != NULL)
{
+ /* An autocmd has freed the location list. */
EMSG(_(e_loc_list_changed));
return FALSE;
}
+ else
+ {
+ /* Quickfix list is not found, create a new one. */
+ qf_new_list(qi, title);
+ return TRUE;
+ }
}
- if (cur_qf_start != qi->qf_lists[qi->qf_curlist].qf_start)
- {
- int idx;
+ if (qi->qf_lists[qi->qf_curlist].qf_id != qfid)
/* Autocommands changed the quickfix list. Find the one we were
* using and restore it. */
- for (idx = 0; idx < LISTCOUNT; ++idx)
- if (cur_qf_start == qi->qf_lists[idx].qf_start)
- {
- qi->qf_curlist = idx;
- break;
- }
- if (idx == LISTCOUNT)
- /* List cannot be found, create a new one. */
- qf_new_list(qi, title);
- }
+ qi->qf_curlist = qf_id2nr(qi, qfid);
return TRUE;
}
@@ -4424,10 +4431,8 @@
char_u *p;
int fi;
qf_info_T *qi = &ql_info;
- int loclist_cmd = FALSE;
int_u save_qfid;
- qfline_T *cur_qf_start;
- win_T *wp;
+ win_T *wp = NULL;
buf_T *buf;
int duplicate_name = FALSE;
int using_dummy;
@@ -4461,7 +4466,7 @@
qi = ll_get_or_alloc_list(curwin);
if (qi == NULL)
return;
- loclist_cmd = TRUE;
+ wp = curwin;
}
if (eap->addr_count > 0)
@@ -4518,10 +4523,9 @@
* ":lcd %:p:h" changes the meaning of short path names. */
mch_dirname(dirname_start, MAXPATHL);
- /* Remember the current values of the quickfix list and qf_start, so that
- * we can check for autocommands changing the current quickfix list. */
+ /* Remember the current quickfix list identifier, so that we can check for
+ * autocommands changing the current quickfix list. */
save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
- cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start;
seconds = (time_t)0;
for (fi = 0; fi < fcount && !got_int && tomatch > 0; ++fi)
@@ -4549,11 +4553,11 @@
/* Use existing, loaded buffer. */
using_dummy = FALSE;
- /* Check whether the quickfix list is still valid */
- if (!vgr_qflist_valid(qi, save_qfid, cur_qf_start, loclist_cmd,
- *eap->cmdlinep))
+ /* Check whether the quickfix list is still valid. When loading a
+ * buffer above, autocommands might have changed the quickfix list. */
+ if (!vgr_qflist_valid(wp, qi, save_qfid, *eap->cmdlinep))
goto theend;
- cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start;
+ save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
if (buf == NULL)
{
@@ -4567,8 +4571,6 @@
found_match = vgr_match_buflines(qi, fname, buf, ®match,
tomatch, duplicate_name, flags);
- cur_qf_start = qi->qf_lists[qi->qf_curlist].qf_start;
-
if (using_dummy)
{
if (found_match && first_match_buf == NULL)
@@ -4649,7 +4651,6 @@
* The QuickFixCmdPost autocmd may free the quickfix list. Check the list
* is still valid.
*/
- wp = loclist_cmd ? curwin : NULL;
if (!qflist_valid(wp, save_qfid))
goto theend;
@@ -4995,21 +4996,6 @@
}
/*
- * Return the quickfix/location list number with the given identifier.
- * Returns -1 if list is not found.
- */
- static int
-qf_id2nr(qf_info_T *qi, int_u qfid)
-{
- int qf_idx;
-
- for (qf_idx = 0; qf_idx < qi->qf_listcount; qf_idx++)
- if (qi->qf_lists[qf_idx].qf_id == qfid)
- return qf_idx;
- return -1;
-}
-
-/*
* Return the quickfix/location list window identifier in the current tabpage.
*/
static int
diff --git a/src/testdir/test_quickfix.vim b/src/testdir/test_quickfix.vim
index 71aa7d5..070582df 100644
--- a/src/testdir/test_quickfix.vim
+++ b/src/testdir/test_quickfix.vim
@@ -3111,3 +3111,44 @@
call assert_equal(3, winnr())
close
endfunc
+
+" Tests for quickfix/location lists changed by autocommands when
+" :vimgrep/:lvimgrep commands are running.
+func Test_vimgrep_autocmd()
+ call setqflist([], 'f')
+ call writefile(['stars'], 'Xtest1.txt')
+ call writefile(['stars'], 'Xtest2.txt')
+
+ " Test 1:
+ " When searching for a pattern using :vimgrep, if the quickfix list is
+ " changed by an autocmd, the results should be added to the correct quickfix
+ " list.
+ autocmd BufRead Xtest2.txt cexpr '' | cexpr ''
+ silent vimgrep stars Xtest*.txt
+ call assert_equal(1, getqflist({'nr' : 0}).nr)
+ call assert_equal(3, getqflist({'nr' : '$'}).nr)
+ call assert_equal('Xtest2.txt', bufname(getqflist()[1].bufnr))
+ au! BufRead Xtest2.txt
+
+ " Test 2:
+ " When searching for a pattern using :vimgrep, if the quickfix list is
+ " freed, then a error should be given.
+ silent! %bwipe!
+ call setqflist([], 'f')
+ autocmd BufRead Xtest2.txt for i in range(10) | cexpr '' | endfor
+ call assert_fails('vimgrep stars Xtest*.txt', 'E925:')
+ au! BufRead Xtest2.txt
+
+ " Test 3:
+ " When searching for a pattern using :lvimgrep, if the location list is
+ " freed, then the command should error out.
+ silent! %bwipe!
+ let g:save_winid = win_getid()
+ autocmd BufRead Xtest2.txt call setloclist(g:save_winid, [], 'f')
+ call assert_fails('lvimgrep stars Xtest*.txt', 'E926:')
+ au! BufRead Xtest2.txt
+
+ call delete('Xtest1.txt')
+ call delete('Xtest2.txt')
+ call setqflist([], 'f')
+endfunc
diff --git a/src/version.c b/src/version.c
index 33bae20..813bcfa 100644
--- a/src/version.c
+++ b/src/version.c
@@ -763,6 +763,8 @@
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
+ 1669,
+/**/
1668,
/**/
1667,