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 f32dcff05a497008164b09dc6cdda84b3c8fcd3c..e5bd2d0f005f52fab7e7ab109acf6940d91e6a75 100644 (file)
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -4159,6 +4159,21 @@ ex_cfile(exarg_T *eap)
        qf_jump(qi, 0, 0, eap->forceit);        /* display first error */
 }
 
+/*
+ * 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.
  */
@@ -4272,40 +4287,32 @@ vgr_load_dummy_buf(
  */
     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 @@ ex_vimgrep(exarg_T *eap)
     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 @@ ex_vimgrep(exarg_T *eap)
        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 @@ ex_vimgrep(exarg_T *eap)
      * ":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 @@ ex_vimgrep(exarg_T *eap)
            /* 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 @@ ex_vimgrep(exarg_T *eap)
            found_match = vgr_match_buflines(qi, fname, buf, &regmatch,
                    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 @@ ex_vimgrep(exarg_T *eap)
      * 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;
 
@@ -4994,21 +4995,6 @@ qf_get_list_from_lines(dict_T *what, dictitem_T *di, dict_T *retdict)
     return status;
 }
 
-/*
- * 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.
  */

diff --git a/src/testdir/test_quickfix.vim b/src/testdir/test_quickfix.vim
index 71aa7d5b192c7f5f49258d4be3521698dab907ce..070582dfbe964461b43b965e14d48e6d6c167f9e 100644 (file)
--- a/src/testdir/test_quickfix.vim
+++ b/src/testdir/test_quickfix.vim
@@ -3111,3 +3111,44 @@ func Test_qfwin_pos()
   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 33bae20366fec25c2b0f3b0ba0dd1e0fec4099d3..813bcfac2cb927cb630f8dae4a20eb1ba356d4d0 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -762,6 +762,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1669,
 /**/
     1668,
 /**/