patch 8.1.0141: :cexpr no longer jumps to the first errorv8.1.0141

Problem:    :cexpr no longer jumps to the first error.
Solution:   Use the quickfix list identifier. (Yegappan Lakshmanan,
            closes #3092)

3 files changed, 206 insertions, 43 deletions

diff --git a/src/quickfix.c b/src/quickfix.c
index d541aba54d..ad25297804 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -2231,7 +2231,7 @@ qflist_valid (win_T *wp, int_u qf_id)
 }
 
 /*
- * When loading a file from the quickfix, the auto commands may modify it.
+ * When loading a file from the quickfix, the autocommands may modify it.
  * This may invalidate the current quickfix entry.  This function checks
  * whether an entry is still present in the quickfix list.
  * Similar to location list.
@@ -2579,7 +2579,8 @@ qf_goto_win_with_ll_file(win_T *use_win, int qf_fnum, qf_info_T *ll_ref)
     if (win->w_llist == NULL)
     {
 	win->w_llist = ll_ref;
-	ll_ref->qf_refcount++;
+	if (ll_ref != NULL)
+	    ll_ref->qf_refcount++;
     }
 }
 
@@ -2986,8 +2987,9 @@ qf_jump(qf_info_T	*qi,
 	if (curbuf == old_curbuf)
 	    setpcmark();
 
-	qf_jump_goto_line(qf_ptr->qf_lnum, qf_ptr->qf_col, qf_ptr->qf_viscol,
-							qf_ptr->qf_pattern);
+	if (qf_ptr != NULL)
+	    qf_jump_goto_line(qf_ptr->qf_lnum, qf_ptr->qf_col,
+				qf_ptr->qf_viscol, qf_ptr->qf_pattern);
 
 #ifdef FEAT_FOLDING
 	if ((fdo_flags & FDO_QUICKFIX) && old_KeyTyped)
@@ -4056,6 +4058,21 @@ qf_list_changed(qf_info_T *qi, int qf_idx)
 }
 
 /*
+ * 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 INVALID_QFIDX;
+}
+
+/*
  * Return TRUE when using ":vimgrep" for ":grep".
  */
     int
@@ -4083,6 +4100,7 @@ ex_make(exarg_T *eap)
     qf_info_T	*qi = &ql_info;
     int		res;
     char_u	*au_name = NULL;
+    int_u	save_qfid;
 
     /* Redirect ":grep" to ":vimgrep" if 'grepprg' is "internal". */
     if (grep_internal(eap->cmdidx))
@@ -4161,21 +4179,28 @@ ex_make(exarg_T *eap)
 					    && eap->cmdidx != CMD_lgrepadd),
 					   qf_cmdtitle(*eap->cmdlinep), enc);
     if (wp != NULL)
+    {
 	qi = GET_LOC_LIST(wp);
-    if (res >= 0 && qi != NULL)
+	if (qi == NULL)
+	    goto cleanup;
+    }
+    if (res >= 0)
 	qf_list_changed(qi, qi->qf_curlist);
+    // 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;
     if (au_name != NULL)
-    {
 	apply_autocmds(EVENT_QUICKFIXCMDPOST, au_name,
 					       curbuf->b_fname, TRUE, curbuf);
-	if (qi != NULL && qi->qf_curlist < qi->qf_listcount)
-	    res = qi->qf_lists[qi->qf_curlist].qf_count;
-	else
-	    res = 0;
-    }
-    if (res > 0 && !eap->forceit)
+    if (res > 0 && !eap->forceit && qflist_valid(wp, save_qfid))
+    {
+	// If autocommands changed the current list, then restore it
+	if (qi->qf_lists[qi->qf_curlist].qf_id != save_qfid)
+	    qi->qf_curlist = qf_id2nr(qi, save_qfid);
 	qf_jump(qi, 0, 0, FALSE);		/* display first error */
+    }
 
+cleanup:
     mch_remove(fname);
     vim_free(fname);
     vim_free(cmd);
@@ -4502,7 +4527,7 @@ ex_cfile(exarg_T *eap)
     win_T	*wp = NULL;
     qf_info_T	*qi = &ql_info;
     char_u	*au_name = NULL;
-    int		save_qfid = 0;  /* init for gcc */
+    int_u	save_qfid = 0;		/* init for gcc */
     int		res;
 
     switch (eap->cmdidx)
@@ -4555,35 +4580,27 @@ ex_cfile(exarg_T *eap)
 			&& eap->cmdidx != CMD_laddfile),
 			qf_cmdtitle(*eap->cmdlinep), enc);
     if (wp != NULL)
+    {
 	qi = GET_LOC_LIST(wp);
-    if (res >= 0 && qi != NULL)
+	if (qi == NULL)
+	    return;
+    }
+    if (res >= 0)
 	qf_list_changed(qi, qi->qf_curlist);
-    if (qi != NULL)
-	save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
+    save_qfid = qi->qf_lists[qi->qf_curlist].qf_id;
     if (au_name != NULL)
 	apply_autocmds(EVENT_QUICKFIXCMDPOST, au_name, NULL, FALSE, curbuf);
 
-    /* An autocmd might have freed the quickfix/location list. Check whether it
-     * is still valid. */
-    if (qi != NULL && !qflist_valid(wp, save_qfid))
-	return;
-    if (res > 0 && (eap->cmdidx == CMD_cfile || eap->cmdidx == CMD_lfile))
+    // Jump to the first error for a new list and if autocmds didn't
+    // free the list.
+    if (res > 0 && (eap->cmdidx == CMD_cfile || eap->cmdidx == CMD_lfile)
+	    && qflist_valid(wp, save_qfid))
+    {
+	// If autocommands changed the current list, then restore it
+	if (qi->qf_lists[qi->qf_curlist].qf_id != save_qfid)
+	    qi->qf_curlist = qf_id2nr(qi, save_qfid);
 	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 INVALID_QFIDX;
+    }
 }
 
 /*
@@ -5070,6 +5087,10 @@ ex_vimgrep(exarg_T *eap)
     if (!qflist_valid(wp, save_qfid))
 	goto theend;
 
+    // If autocommands changed the current list, then restore it
+    if (qi->qf_lists[qi->qf_curlist].qf_id != save_qfid)
+	qi->qf_curlist = qf_id2nr(qi, save_qfid);
+
     /* Jump to first match. */
     if (qi->qf_lists[qi->qf_curlist].qf_count > 0)
     {
@@ -5162,7 +5183,7 @@ load_dummy_buffer(
     /* need to open the memfile before putting the buffer in a window */
     if (ml_open(newbuf) == OK)
     {
-	/* Make sure this buffer isn't wiped out by auto commands. */
+	/* Make sure this buffer isn't wiped out by autocommands. */
 	++newbuf->b_locked;
 
 	/* set curwin/curbuf to buf and save a few things */
@@ -6205,6 +6226,8 @@ ex_cbuffer(exarg_T *eap)
     qf_info_T	*qi = &ql_info;
     char_u	*au_name = NULL;
     int		res;
+    int_u	save_qfid;
+    win_T	*wp = NULL;
 
     switch (eap->cmdidx)
     {
@@ -6233,6 +6256,7 @@ ex_cbuffer(exarg_T *eap)
 	qi = ll_get_or_alloc_list(curwin);
 	if (qi == NULL)
 	    return;
+	wp = curwin;
     }
 
     if (*eap->arg == NUL)
@@ -6271,6 +6295,10 @@ ex_cbuffer(exarg_T *eap)
 						   qf_title, NULL);
 	    if (res >= 0)
 		qf_list_changed(qi, qi->qf_curlist);
+
+	    // 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;
 	    if (au_name != NULL)
 	    {
 		buf_T *curbuf_old = curbuf;
@@ -6282,9 +6310,17 @@ ex_cbuffer(exarg_T *eap)
 		    // be invalid.
 		    res = 0;
 	    }
+	    // Jump to the first error for a new list and if autocmds didn't
+	    // free the list.
 	    if (res > 0 && (eap->cmdidx == CMD_cbuffer ||
-						eap->cmdidx == CMD_lbuffer))
+						eap->cmdidx == CMD_lbuffer)
+		    && qflist_valid(wp, save_qfid))
+	    {
+		// If autocommands changed the current list, then restore it
+		if (qi->qf_lists[qi->qf_curlist].qf_id != save_qfid)
+		    qi->qf_curlist = qf_id2nr(qi, save_qfid);
 		qf_jump(qi, 0, 0, eap->forceit);  /* display first error */
+	    }
 	}
     }
 }
@@ -6301,6 +6337,8 @@ ex_cexpr(exarg_T *eap)
     qf_info_T	*qi = &ql_info;
     char_u	*au_name = NULL;
     int		res;
+    int_u	save_qfid;
+    win_T	*wp = NULL;
 
     switch (eap->cmdidx)
     {
@@ -6328,6 +6366,7 @@ ex_cexpr(exarg_T *eap)
 	qi = ll_get_or_alloc_list(curwin);
 	if (qi == NULL)
 	    return;
+	wp = curwin;
     }
 
     /* Evaluate the expression.  When the result is a string or a list we can
@@ -6345,14 +6384,25 @@ ex_cexpr(exarg_T *eap)
 				 qf_cmdtitle(*eap->cmdlinep), NULL);
 	    if (res >= 0)
 		qf_list_changed(qi, qi->qf_curlist);
+
+	    // 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;
 	    if (au_name != NULL)
 		apply_autocmds(EVENT_QUICKFIXCMDPOST, au_name,
 						curbuf->b_fname, TRUE, curbuf);
+
+	    // Jump to the first error for a new list and if autocmds didn't
+	    // free the list.
 	    if (res > 0 && (eap->cmdidx == CMD_cexpr
 						   || eap->cmdidx == CMD_lexpr)
-		    && qi == GET_LOC_LIST(curwin))
-		// Jump to the first error if autocmds didn't free the list.
+		    && qflist_valid(wp, save_qfid))
+	    {
+		// If autocommands changed the current list, then restore it
+		if (qi->qf_lists[qi->qf_curlist].qf_id != save_qfid)
+		    qi->qf_curlist = qf_id2nr(qi, save_qfid);
 		qf_jump(qi, 0, 0, eap->forceit);
+	    }
 	}
 	else
 	    EMSG(_("E777: String or List expected"));
diff --git a/src/testdir/test_quickfix.vim b/src/testdir/test_quickfix.vim
index 80f327d64a..5e8b993962 100644
--- a/src/testdir/test_quickfix.vim
+++ b/src/testdir/test_quickfix.vim
@@ -3363,13 +3363,124 @@ func Test_lbuffer_with_bwipe()
   augroup END
 endfunc
 
-func Test_setloclist_in_aucmd()
+" Test for an autocmd freeing the quickfix/location list when cexpr/lexpr is
+" running
+func Xexpr_acmd_freelist(cchar)
+  call s:setup_commands(a:cchar)
+
   " This was using freed memory.
   augroup nasty
-    au * * call setloclist(0, [], 'f')
+    au * * call g:Xsetlist([], 'f')
   augroup END
-  lexpr "x"
+  Xexpr "x"
   augroup nasty
     au!
   augroup END
 endfunc
+
+func Test_cexpr_acmd_freelist()
+  call Xexpr_acmd_freelist('c')
+  call Xexpr_acmd_freelist('l')
+endfunc
+
+" Test for commands that create a new quickfix/location list and jump to the
+" first error automatically.
+func Xjumpto_first_error_test(cchar)
+  call s:setup_commands(a:cchar)
+
+  call s:create_test_file('Xtestfile1')
+  call s:create_test_file('Xtestfile2')
+  let l = ['Xtestfile1:2:Line2', 'Xtestfile2:4:Line4']
+
+  " Test for cexpr/lexpr
+  enew
+  Xexpr l
+  call assert_equal('Xtestfile1', bufname(''))
+  call assert_equal(2, line('.'))
+
+  " Test for cfile/lfile
+  enew
+  call writefile(l, 'Xerr')
+  Xfile Xerr
+  call assert_equal('Xtestfile1', bufname(''))
+  call assert_equal(2, line('.'))
+
+  " Test for cbuffer/lbuffer
+  edit Xerr
+  Xbuffer
+  call assert_equal('Xtestfile1', bufname(''))
+  call assert_equal(2, line('.'))
+
+  call delete('Xerr')
+  call delete('Xtestfile1')
+  call delete('Xtestfile2')
+endfunc
+
+func Test_jumpto_first_error()
+  call Xjumpto_first_error_test('c')
+  call Xjumpto_first_error_test('l')
+endfunc
+
+" Test for a quickfix autocmd changing the quickfix/location list before
+" jumping to the first error in the new list.
+func Xautocmd_changelist(cchar)
+  call s:setup_commands(a:cchar)
+
+  " Test for cfile/lfile
+  call s:create_test_file('Xtestfile1')
+  call s:create_test_file('Xtestfile2')
+  Xexpr 'Xtestfile1:2:Line2'
+  autocmd QuickFixCmdPost * Xolder
+  call writefile(['Xtestfile2:4:Line4'], 'Xerr')
+  Xfile Xerr
+  call assert_equal('Xtestfile2', bufname(''))
+  call assert_equal(4, line('.'))
+  autocmd! QuickFixCmdPost
+
+  " Test for cbuffer/lbuffer
+  call g:Xsetlist([], 'f')
+  Xexpr 'Xtestfile1:2:Line2'
+  autocmd QuickFixCmdPost * Xolder
+  call writefile(['Xtestfile2:4:Line4'], 'Xerr')
+  edit Xerr
+  Xbuffer
+  call assert_equal('Xtestfile2', bufname(''))
+  call assert_equal(4, line('.'))
+  autocmd! QuickFixCmdPost
+
+  " Test for cexpr/lexpr
+  call g:Xsetlist([], 'f')
+  Xexpr 'Xtestfile1:2:Line2'
+  autocmd QuickFixCmdPost * Xolder
+  Xexpr 'Xtestfile2:4:Line4'
+  call assert_equal('Xtestfile2', bufname(''))
+  call assert_equal(4, line('.'))
+  autocmd! QuickFixCmdPost
+
+  " Test for grep/lgrep
+  call g:Xsetlist([], 'f')
+  Xexpr 'Xtestfile1:2:Line2'
+  autocmd QuickFixCmdPost * Xolder
+  silent Xgrep Line5 Xtestfile2
+  call assert_equal('Xtestfile2', bufname(''))
+  call assert_equal(5, line('.'))
+  autocmd! QuickFixCmdPost
+
+  " Test for vimgrep/lvimgrep
+  call g:Xsetlist([], 'f')
+  Xexpr 'Xtestfile1:2:Line2'
+  autocmd QuickFixCmdPost * Xolder
+  silent Xvimgrep Line5 Xtestfile2
+  call assert_equal('Xtestfile2', bufname(''))
+  call assert_equal(5, line('.'))
+  autocmd! QuickFixCmdPost
+
+  call delete('Xerr')
+  call delete('Xtestfile1')
+  call delete('Xtestfile2')
+endfunc
+
+func Test_autocmd_changelist()
+  call Xautocmd_changelist('c')
+  call Xautocmd_changelist('l')
+endfunc
diff --git a/src/version.c b/src/version.c
index 512001c1e2..ff1297ab61 100644
--- a/src/version.c
+++ b/src/version.c
@@ -790,6 +790,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    141,
+/**/
     140,
 /**/
     139,