patch 8.1.0431: the qf_jump() function is too longv8.1.0431

Problem:    The qf_jump() function is too long.
Solution:   Refactor to split it into several functions. (Yegappan Lakshmanan)

2 files changed, 162 insertions, 105 deletions

diff --git a/src/quickfix.c b/src/quickfix.c
index 9c346519fa..59b3a227c8 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -2521,10 +2521,11 @@ get_prev_valid_entry(
 get_nth_valid_entry(
 	qf_list_T	*qfl,
 	int		errornr,
-	qfline_T	*qf_ptr,
-	int		*qf_index,
-	int		dir)
+	int		dir,
+	int		*new_qfidx)
 {
+    qfline_T		*qf_ptr = qfl->qf_ptr;
+    int			qf_idx = qfl->qf_index;
     qfline_T		*prev_qf_ptr;
     int			prev_index;
     static char_u	*e_no_more_items = (char_u *)N_("E553: No more items");
@@ -2533,16 +2534,16 @@ get_nth_valid_entry(
     while (errornr--)
     {
 	prev_qf_ptr = qf_ptr;
-	prev_index = *qf_index;
+	prev_index = qf_idx;
 
 	if (dir == FORWARD || dir == FORWARD_FILE)
-	    qf_ptr = get_next_valid_entry(qfl, qf_ptr, qf_index, dir);
+	    qf_ptr = get_next_valid_entry(qfl, qf_ptr, &qf_idx, dir);
 	else
-	    qf_ptr = get_prev_valid_entry(qfl, qf_ptr, qf_index, dir);
+	    qf_ptr = get_prev_valid_entry(qfl, qf_ptr, &qf_idx, dir);
 	if (qf_ptr == NULL)
 	{
 	    qf_ptr = prev_qf_ptr;
-	    *qf_index = prev_index;
+	    qf_idx = prev_index;
 	    if (err != NULL)
 	    {
 		EMSG(_(err));
@@ -2554,28 +2555,27 @@ get_nth_valid_entry(
 	err = NULL;
     }
 
+    *new_qfidx = qf_idx;
     return qf_ptr;
 }
 
 /*
- * Get n'th (errornr) quickfix entry
+ * Get n'th (errornr) quickfix entry from the current entry in the quickfix
+ * list 'qfl'. Returns a pointer to the new entry and the index in 'new_qfidx'
  */
     static qfline_T *
-get_nth_entry(
-	qf_list_T	*qfl,
-	int		errornr,
-	qfline_T	*qf_ptr,
-	int		*cur_qfidx)
+get_nth_entry(qf_list_T *qfl, int errornr, int *new_qfidx)
 {
-    int		qf_idx = *cur_qfidx;
+    qfline_T	*qf_ptr = qfl->qf_ptr;
+    int		qf_idx = qfl->qf_index;
 
-    /* New error number is less than the current error number */
+    // New error number is less than the current error number
     while (errornr < qf_idx && qf_idx > 1 && qf_ptr->qf_prev != NULL)
     {
 	--qf_idx;
 	qf_ptr = qf_ptr->qf_prev;
     }
-    /* New error number is greater than the current error number */
+    // New error number is greater than the current error number
     while (errornr > qf_idx && qf_idx < qfl->qf_count &&
 						qf_ptr->qf_next != NULL)
     {
@@ -2583,7 +2583,33 @@ get_nth_entry(
 	qf_ptr = qf_ptr->qf_next;
     }
 
-    *cur_qfidx = qf_idx;
+    *new_qfidx = qf_idx;
+    return qf_ptr;
+}
+
+/*
+ * Get a entry specied by 'errornr' and 'dir' from the current
+ * quickfix/location list. 'errornr' specifies the index of the entry and 'dir'
+ * specifies the direction (FORWARD/BACKWARD/FORWARD_FILE/BACKWARD_FILE).
+ * Returns a pointer to the entry and the index of the new entry is stored in
+ * 'new_qfidx'.
+ */
+    static qfline_T *
+qf_get_entry(
+	qf_list_T	*qfl,
+	int		errornr,
+	int		dir,
+	int		*new_qfidx)
+{
+    qfline_T	*qf_ptr = qfl->qf_ptr;
+    int		qfidx = qfl->qf_index;
+
+    if (dir != 0)    // next/prev valid entry
+	qf_ptr = get_nth_valid_entry(qfl, errornr, dir, &qfidx);
+    else if (errornr != 0)	// go to specified number
+	qf_ptr = get_nth_entry(qfl, errornr, &qfidx);
+
+    *new_qfidx = qfidx;
     return qf_ptr;
 }
 
@@ -2881,6 +2907,9 @@ qf_jump_to_usable_window(int qf_fnum, int *opened_window)
 
 /*
  * Edit the selected file or help file.
+ * Returns OK if successfully edited the file, FAIL on failing to open the
+ * buffer and NOTDONE if the quickfix/location list was freed by an autocmd
+ * when opening the buffer.
  */
     static int
 qf_jump_edit_buffer(
@@ -2888,8 +2917,7 @@ qf_jump_edit_buffer(
 	qfline_T	*qf_ptr,
 	int		forceit,
 	win_T		*oldwin,
-	int		*opened_window,
-	int		*abort)
+	int		*opened_window)
 {
     qf_list_T	*qfl = &qi->qf_lists[qi->qf_curlist];
     int		retval = OK;
@@ -2925,13 +2953,13 @@ qf_jump_edit_buffer(
 	    if (!win_valid_any_tab(oldwin))
 	    {
 		EMSG(_("E924: Current window was closed"));
-		*abort = TRUE;
 		*opened_window = FALSE;
+		return NOTDONE;
 	    }
 	    else if (!qflist_valid(oldwin, save_qfid))
 	    {
 		EMSG(_(e_loc_list_changed));
-		*abort = TRUE;
+		return NOTDONE;
 	    }
 	}
 	else if (old_qf_curlist != qi->qf_curlist
@@ -2941,11 +2969,8 @@ qf_jump_edit_buffer(
 		EMSG(_("E925: Current quickfix was changed"));
 	    else
 		EMSG(_(e_loc_list_changed));
-	    *abort = TRUE;
+	    return NOTDONE;
 	}
-
-	if (*abort)
-	    retval = FAIL;
     }
 
     return retval;
@@ -3066,6 +3091,90 @@ qf_jump_print_msg(
 }
 
 /*
+ * Find a usable window for opening a file from the quickfix/location list. If
+ * a window is not found then open a new window.
+ * Returns OK if successfully jumped or opened a window. Returns FAIL if not
+ * able to jump/open a window.  Returns NOTDONE if a file is not associated
+ * with the entry.
+ */
+    static int
+qf_jump_open_window(qf_info_T *qi, qfline_T *qf_ptr, int *opened_window)
+{
+    // For ":helpgrep" find a help window or open one.
+    if (qf_ptr->qf_type == 1 && (!bt_help(curwin->w_buffer) || cmdmod.tab != 0))
+	if (jump_to_help_window(qi, opened_window) == FAIL)
+	    return FAIL;
+
+    // If currently in the quickfix window, find another window to show the
+    // file in.
+    if (bt_quickfix(curbuf) && !*opened_window)
+    {
+	// If there is no file specified, we don't know where to go.
+	// But do advance, otherwise ":cn" gets stuck.
+	if (qf_ptr->qf_fnum == 0)
+	    return NOTDONE;
+
+	if (qf_jump_to_usable_window(qf_ptr->qf_fnum, opened_window) == FAIL)
+	    return FAIL;
+    }
+
+    return OK;
+}
+
+/*
+ * Edit a selected file from the quickfix/location list and jump to a
+ * particular line/column, adjust the folds and display a message about the
+ * jump.
+ * Returns OK on success and FAIL on failing to open the file/buffer.  Returns
+ * NOTDONE if the quickfix/location list is freed by an autocmd when opening
+ * the file.
+ */
+    static int
+qf_jump_to_buffer(
+	qf_info_T	*qi,
+	int		qf_index,
+	qfline_T	*qf_ptr,
+	int		forceit,
+	win_T		*oldwin,
+	int		*opened_window,
+	int		openfold,
+	int		print_message)
+{
+    buf_T	*old_curbuf;
+    linenr_T	old_lnum;
+    int		retval = OK;
+
+    // If there is a file name, read the wanted file if needed, and check
+    // autowrite etc.
+    old_curbuf = curbuf;
+    old_lnum = curwin->w_cursor.lnum;
+
+    if (qf_ptr->qf_fnum != 0)
+    {
+	retval = qf_jump_edit_buffer(qi, qf_ptr, forceit, oldwin,
+						opened_window);
+	if (retval != OK)
+	    return retval;
+    }
+
+    // When not switched to another buffer, still need to set pc mark
+    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);
+
+#ifdef FEAT_FOLDING
+    if ((fdo_flags & FDO_QUICKFIX) && openfold)
+	foldOpenCursor();
+#endif
+    if (print_message)
+	qf_jump_print_msg(qi, qf_index, qf_ptr, old_curbuf, old_lnum);
+
+    return retval;
+}
+
+/*
  * jump to a quickfix line
  * if dir == FORWARD go "errornr" valid entries forward
  * if dir == BACKWARD go "errornr" valid entries backward
@@ -3085,15 +3194,13 @@ qf_jump(qf_info_T	*qi,
     qfline_T		*old_qf_ptr;
     int			qf_index;
     int			old_qf_index;
-    buf_T		*old_curbuf;
-    linenr_T		old_lnum;
     char_u		*old_swb = p_swb;
     unsigned		old_swb_flags = swb_flags;
     int			opened_window = FALSE;
     win_T		*oldwin = curwin;
     int			print_message = TRUE;
 #ifdef FEAT_FOLDING
-    int			old_KeyTyped = KeyTyped; /* getting file may reset it */
+    int			old_KeyTyped = KeyTyped; // getting file may reset it
 #endif
     int			retval = OK;
 
@@ -3113,96 +3220,44 @@ qf_jump(qf_info_T	*qi,
     old_qf_ptr = qf_ptr;
     qf_index = qfl->qf_index;
     old_qf_index = qf_index;
-    if (dir != 0)    /* next/prev valid entry */
+
+    qf_ptr = qf_get_entry(qfl, errornr, dir, &qf_index);
+    if (qf_ptr == NULL)
     {
-	qf_ptr = get_nth_valid_entry(qfl, errornr, qf_ptr, &qf_index, dir);
-	if (qf_ptr == NULL)
-	{
-	    qf_ptr = old_qf_ptr;
-	    qf_index = old_qf_index;
-	    goto theend;
-	}
+	qf_ptr = old_qf_ptr;
+	qf_index = old_qf_index;
+	goto theend;
     }
-    else if (errornr != 0)	/* go to specified number */
-	qf_ptr = get_nth_entry(qfl, errornr, qf_ptr, &qf_index);
 
     qfl->qf_index = qf_index;
     if (qf_win_pos_update(qi, old_qf_index))
-	/* No need to print the error message if it's visible in the error
-	 * window */
+	// No need to print the error message if it's visible in the error
+	// window
 	print_message = FALSE;
 
-    /*
-     * For ":helpgrep" find a help window or open one.
-     */
-    if (qf_ptr->qf_type == 1 && (!bt_help(curwin->w_buffer) || cmdmod.tab != 0))
-	if (jump_to_help_window(qi, &opened_window) == FAIL)
-	    goto theend;
-
-    /*
-     * If currently in the quickfix window, find another window to show the
-     * file in.
-     */
-    if (bt_quickfix(curbuf) && !opened_window)
-    {
-	/*
-	 * If there is no file specified, we don't know where to go.
-	 * But do advance, otherwise ":cn" gets stuck.
-	 */
-	if (qf_ptr->qf_fnum == 0)
-	    goto theend;
-
-	if (qf_jump_to_usable_window(qf_ptr->qf_fnum, &opened_window) == FAIL)
-	    goto failed;
-    }
-
-    /*
-     * If there is a file name,
-     * read the wanted file if needed, and check autowrite etc.
-     */
-    old_curbuf = curbuf;
-    old_lnum = curwin->w_cursor.lnum;
+    retval = qf_jump_open_window(qi, qf_ptr, &opened_window);
+    if (retval == FAIL)
+	goto failed;
+    if (retval == NOTDONE)
+	goto theend;
 
-    if (qf_ptr->qf_fnum != 0)
+    retval = qf_jump_to_buffer(qi, qf_index, qf_ptr, forceit, oldwin,
+	    &opened_window, old_KeyTyped, print_message);
+    if (retval == NOTDONE)
     {
-	int abort = FALSE;
-
-	retval = qf_jump_edit_buffer(qi, qf_ptr, forceit, oldwin,
-						&opened_window, &abort);
-	if (abort)
-	{
-	    qi = NULL;
-	    qf_ptr = NULL;
-	}
+	// Quickfix/location list is freed by an autocmd
+	qi = NULL;
+	qf_ptr = NULL;
     }
 
-    if (retval == OK)
-    {
-	/* When not switched to another buffer, still need to set pc mark */
-	if (curbuf == old_curbuf)
-	    setpcmark();
-
-	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)
-	    foldOpenCursor();
-#endif
-	if (print_message)
-	    qf_jump_print_msg(qi, qf_index, qf_ptr, old_curbuf, old_lnum);
-    }
-    else
+    if (retval != OK)
     {
 	if (opened_window)
-	    win_close(curwin, TRUE);    /* Close opened window */
+	    win_close(curwin, TRUE);    // Close opened window
 	if (qf_ptr != NULL && qf_ptr->qf_fnum != 0)
 	{
-	    /*
-	     * Couldn't open file, so put index back where it was.  This could
-	     * happen if the file was readonly and we changed something.
-	     */
+	    // Couldn't open file, so put index back where it was.  This could
+	    // happen if the file was readonly and we changed something.
 failed:
 	    qf_ptr = old_qf_ptr;
 	    qf_index = old_qf_index;
@@ -3216,8 +3271,8 @@ theend:
     }
     if (p_swb != old_swb && opened_window)
     {
-	/* Restore old 'switchbuf' value, but not when an autocommand or
-	 * modeline has changed the value. */
+	// Restore old 'switchbuf' value, but not when an autocommand or
+	// modeline has changed the value.
 	if (p_swb == empty_option)
 	{
 	    p_swb = old_swb;
diff --git a/src/version.c b/src/version.c
index 7ca18bc16a..1d3dc43699 100644
--- a/src/version.c
+++ b/src/version.c
@@ -795,6 +795,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    431,
+/**/
     430,
 /**/
     429,