patch 9.0.0388: the do_arg_all() function is too longv9.0.0388

Problem:    The do_arg_all() function is too long.
Solution:   Split the function in smaller parts. (Yegappan Lakshmanan,
            closes #11062)

1 files changed, 179 insertions, 127 deletions

diff --git a/src/arglist.c b/src/arglist.c
index d6e613f0b5..e44682fa4c 100644
--- a/src/arglist.c
+++ b/src/arglist.c
@@ -574,21 +574,23 @@ ex_args(exarg_T *eap)
 	    alist_new();
     }
 
+    // ":args file ..": define new argument list, handle like ":next"
+    // Also for ":argslocal file .." and ":argsglobal file ..".
     if (*eap->arg != NUL)
     {
 	if (check_arglist_locked() == FAIL)
 	    return;
-	// ":args file ..": define new argument list, handle like ":next"
-	// Also for ":argslocal file .." and ":argsglobal file ..".
 	ex_next(eap);
+	return;
     }
-    else if (eap->cmdidx == CMD_args)
+
+    // ":args": list arguments.
+    if (eap->cmdidx == CMD_args)
     {
 	char_u **items;
 
-	// ":args": list arguments.
 	if (ARGCOUNT <= 0)
-	    return;
+	    return;		// empty argument list
 
 	items = ALLOC_MULT(char_u *, ARGCOUNT);
 	if (items == NULL)
@@ -602,12 +604,15 @@ ex_args(exarg_T *eap)
 	    items[i] = alist_name(&ARGLIST[i]);
 	list_in_columns(items, ARGCOUNT, curwin->w_arg_idx);
 	vim_free(items);
+
+	return;
     }
-    else if (eap->cmdidx == CMD_arglocal)
+
+    // ":argslocal": make a local copy of the global argument list.
+    if (eap->cmdidx == CMD_arglocal)
     {
 	garray_T	*gap = &curwin->w_alist->al_ga;
 
-	// ":argslocal": make a local copy of the global argument list.
 	if (GA_GROW_FAILS(gap, GARGCOUNT))
 	    return;
 
@@ -919,77 +924,45 @@ alist_name(aentry_T *aep)
 }
 
 /*
- * do_arg_all(): Open up to 'count' windows, one for each argument.
+ * State used by the :all command to open all the files in the argument list in
+ * separate windows.
  */
-    static void
-do_arg_all(
-    int	count,
-    int	forceit,		// hide buffers in current windows
-    int keep_tabs)		// keep current tabs, for ":tab drop file"
-{
-    int		i;
-    win_T	*wp, *wpnext;
+typedef struct {
+    alist_T	*alist;		// argument list to be used
+    int		had_tab;
+    int		keep_tabs;
+    int		forceit;
+
+    int		use_firstwin;	// use first window for arglist
     char_u	*opened;	// Array of weight for which args are open:
 				//  0: not opened
 				//  1: opened in other tab
 				//  2: opened in curtab
 				//  3: opened in curtab and curwin
-				//
     int		opened_len;	// length of opened[]
-    int		use_firstwin = FALSE;	// use first window for arglist
-    int		tab_drop_empty_window = FALSE;
-    int		split_ret = OK;
-    int		p_ea_save;
-    alist_T	*alist;		// argument list to be used
-    buf_T	*buf;
-    tabpage_T	*tpnext;
-    int		had_tab = cmdmod.cmod_tab;
-    win_T	*old_curwin, *last_curwin;
-    tabpage_T	*old_curtab, *last_curtab;
-    win_T	*new_curwin = NULL;
-    tabpage_T	*new_curtab = NULL;
-    int		prev_arglist_locked = arglist_locked;
-
-#ifdef FEAT_CMDWIN
-    if (cmdwin_type != 0)
-    {
-	emsg(_(e_invalid_in_cmdline_window));
-	return;
-    }
-#endif
-    if (ARGCOUNT <= 0)
-    {
-	// Don't give an error message.  We don't want it when the ":all"
-	// command is in the .vimrc.
-	return;
-    }
-    setpcmark();
+    win_T	*new_curwin;
+    tabpage_T	*new_curtab;
+} arg_all_state_T;
 
-    opened_len = ARGCOUNT;
-    opened = alloc_clear(opened_len);
-    if (opened == NULL)
-	return;
-
-    // Autocommands may do anything to the argument list.  Make sure it's not
-    // freed while we are working here by "locking" it.  We still have to
-    // watch out for its size to be changed.
-    alist = curwin->w_alist;
-    ++alist->al_refcount;
-    arglist_locked = TRUE;
+/*
+ * Close all the windows containing files which are not in the argument list.
+ * Used by the ":all" command.
+ */
+    static void
+arg_all_close_unused_windows(arg_all_state_T *aall)
+{
+    win_T	*wp;
+    win_T	*wpnext;
+    tabpage_T	*tpnext;
+    buf_T	*buf;
+    int		i;
+    win_T	*old_curwin;
+    tabpage_T	*old_curtab;
 
     old_curwin = curwin;
     old_curtab = curtab;
 
-#ifdef FEAT_GUI
-    need_mouse_correct = TRUE;
-#endif
-
-    // Try closing all windows that are not in the argument list.
-    // Also close windows that are not full width;
-    // When 'hidden' or "forceit" set the buffer becomes hidden.
-    // Windows that have a changed buffer and can't be hidden won't be closed.
-    // When the ":tab" modifier was used do this for all tab pages.
-    if (had_tab > 0)
+    if (aall->had_tab > 0)
 	goto_tabpage_tp(first_tabpage, TRUE, TRUE);
     for (;;)
     {
@@ -999,17 +972,17 @@ do_arg_all(
 	    wpnext = wp->w_next;
 	    buf = wp->w_buffer;
 	    if (buf->b_ffname == NULL
-		    || (!keep_tabs && (buf->b_nwindows > 1
+		    || (!aall->keep_tabs && (buf->b_nwindows > 1
 			    || wp->w_width != Columns)))
-		i = opened_len;
+		i = aall->opened_len;
 	    else
 	    {
 		// check if the buffer in this window is in the arglist
-		for (i = 0; i < opened_len; ++i)
+		for (i = 0; i < aall->opened_len; ++i)
 		{
-		    if (i < alist->al_ga.ga_len
-			    && (AARGLIST(alist)[i].ae_fnum == buf->b_fnum
-				|| fullpathcmp(alist_name(&AARGLIST(alist)[i]),
+		    if (i < aall->alist->al_ga.ga_len
+			    && (AARGLIST(aall->alist)[i].ae_fnum == buf->b_fnum
+				|| fullpathcmp(alist_name(&AARGLIST(aall->alist)[i]),
 					buf->b_ffname, TRUE, TRUE) & FPC_SAME))
 		    {
 			int weight = 1;
@@ -1021,26 +994,26 @@ do_arg_all(
 				++weight;
 			}
 
-			if (weight > (int)opened[i])
+			if (weight > (int)aall->opened[i])
 			{
-			    opened[i] = (char_u)weight;
+			    aall->opened[i] = (char_u)weight;
 			    if (i == 0)
 			    {
-				if (new_curwin != NULL)
-				    new_curwin->w_arg_idx = opened_len;
-				new_curwin = wp;
-				new_curtab = curtab;
+				if (aall->new_curwin != NULL)
+				    aall->new_curwin->w_arg_idx = aall->opened_len;
+				aall->new_curwin = wp;
+				aall->new_curtab = curtab;
 			    }
 			}
-			else if (keep_tabs)
-			    i = opened_len;
+			else if (aall->keep_tabs)
+			    i = aall->opened_len;
 
-			if (wp->w_alist != alist)
+			if (wp->w_alist != aall->alist)
 			{
 			    // Use the current argument list for all windows
 			    // containing a file from it.
 			    alist_unlink(wp->w_alist);
-			    wp->w_alist = alist;
+			    wp->w_alist = aall->alist;
 			    ++wp->w_alist->al_refcount;
 			}
 			break;
@@ -1049,9 +1022,9 @@ do_arg_all(
 	    }
 	    wp->w_arg_idx = i;
 
-	    if (i == opened_len && !keep_tabs)// close this window
+	    if (i == aall->opened_len && !aall->keep_tabs)// close this window
 	    {
-		if (buf_hide(buf) || forceit || buf->b_nwindows > 1
+		if (buf_hide(buf) || aall->forceit || buf->b_nwindows > 1
 							|| !bufIsChanged(buf))
 		{
 		    // If the buffer was changed, and we would like to hide it,
@@ -1074,8 +1047,9 @@ do_arg_all(
 		    }
 		    // don't close last window
 		    if (ONE_WINDOW
-			    && (first_tabpage->tp_next == NULL || !had_tab))
-			use_firstwin = TRUE;
+			    && (first_tabpage->tp_next == NULL
+				|| !aall->had_tab))
+			aall->use_firstwin = TRUE;
 		    else
 		    {
 			win_close(wp, !buf_hide(buf) && !bufIsChanged(buf));
@@ -1089,7 +1063,7 @@ do_arg_all(
 	}
 
 	// Without the ":tab" modifier only do the current tab page.
-	if (had_tab == 0 || tpnext == NULL)
+	if (aall->had_tab == 0 || tpnext == NULL)
 	    break;
 
 	// check if autocommands removed the next tab page
@@ -1098,54 +1072,56 @@ do_arg_all(
 
 	goto_tabpage_tp(tpnext, TRUE, TRUE);
     }
+}
 
-    // Open a window for files in the argument list that don't have one.
-    // ARGCOUNT may change while doing this, because of autocommands.
-    if (count > opened_len || count <= 0)
-	count = opened_len;
+/*
+ * Open upto "count" windows for the files in the argument list 'aall->alist'.
+ */
+    static void
+arg_all_open_windows(arg_all_state_T *aall, int count)
+{
+    win_T	*wp;
+    int		tab_drop_empty_window = FALSE;
+    int		i;
+    int		split_ret = OK;
+    int		p_ea_save;
 
-    // Don't execute Win/Buf Enter/Leave autocommands here.
-    ++autocmd_no_enter;
-    ++autocmd_no_leave;
-    last_curwin = curwin;
-    last_curtab = curtab;
-    win_enter(lastwin, FALSE);
     // ":tab drop file" should re-use an empty window to avoid "--remote-tab"
     // leaving an empty tab page when executed locally.
-    if (keep_tabs && BUFEMPTY() && curbuf->b_nwindows == 1
+    if (aall->keep_tabs && BUFEMPTY() && curbuf->b_nwindows == 1
 			    && curbuf->b_ffname == NULL && !curbuf->b_changed)
     {
-	use_firstwin = TRUE;
+	aall->use_firstwin = TRUE;
 	tab_drop_empty_window = TRUE;
     }
 
     for (i = 0; i < count && !got_int; ++i)
     {
-	if (alist == &global_alist && i == global_alist.al_ga.ga_len - 1)
+	if (aall->alist == &global_alist && i == global_alist.al_ga.ga_len - 1)
 	    arg_had_last = TRUE;
-	if (opened[i] > 0)
+	if (aall->opened[i] > 0)
 	{
 	    // Move the already present window to below the current window
 	    if (curwin->w_arg_idx != i)
 	    {
-		FOR_ALL_WINDOWS(wpnext)
+		FOR_ALL_WINDOWS(wp)
 		{
-		    if (wpnext->w_arg_idx == i)
+		    if (wp->w_arg_idx == i)
 		    {
-			if (keep_tabs)
+			if (aall->keep_tabs)
 			{
-			    new_curwin = wpnext;
-			    new_curtab = curtab;
+			    aall->new_curwin = wp;
+			    aall->new_curtab = curtab;
 			}
-			else if (wpnext->w_frame->fr_parent
-						 != curwin->w_frame->fr_parent)
+			else if (wp->w_frame->fr_parent
+				!= curwin->w_frame->fr_parent)
 			{
 			    emsg(_(e_window_layout_changed_unexpectedly));
 			    i = count;
 			    break;
 			}
 			else
-			    win_move_after(wpnext, curwin);
+			    win_move_after(wp, curwin);
 			break;
 		    }
 		}
@@ -1156,7 +1132,7 @@ do_arg_all(
 	    // trigger events for tab drop
 	    if (tab_drop_empty_window && i == count - 1)
 		--autocmd_no_enter;
-	    if (!use_firstwin)		// split current window
+	    if (!aall->use_firstwin)		// split current window
 	    {
 		p_ea_save = p_ea;
 		p_ea = TRUE;		// use space from all windows
@@ -1172,35 +1148,111 @@ do_arg_all(
 	    curwin->w_arg_idx = i;
 	    if (i == 0)
 	    {
-		new_curwin = curwin;
-		new_curtab = curtab;
+		aall->new_curwin = curwin;
+		aall->new_curtab = curtab;
 	    }
-	    (void)do_ecmd(0, alist_name(&AARGLIST(alist)[i]), NULL, NULL,
-		      ECMD_ONE,
-		      ((buf_hide(curwin->w_buffer)
-			   || bufIsChanged(curwin->w_buffer)) ? ECMD_HIDE : 0)
-						       + ECMD_OLDBUF, curwin);
+	    (void)do_ecmd(0, alist_name(&AARGLIST(aall->alist)[i]), NULL, NULL,
+		    ECMD_ONE,
+		    ((buf_hide(curwin->w_buffer)
+		      || bufIsChanged(curwin->w_buffer)) ? ECMD_HIDE : 0)
+		    + ECMD_OLDBUF, curwin);
 	    if (tab_drop_empty_window && i == count - 1)
 		++autocmd_no_enter;
-	    if (use_firstwin)
+	    if (aall->use_firstwin)
 		++autocmd_no_leave;
-	    use_firstwin = FALSE;
+	    aall->use_firstwin = FALSE;
 	}
 	ui_breakcheck();
 
 	// When ":tab" was used open a new tab for a new window repeatedly.
-	if (had_tab > 0 && tabpage_index(NULL) <= p_tpm)
+	if (aall->had_tab > 0 && tabpage_index(NULL) <= p_tpm)
 	    cmdmod.cmod_tab = 9999;
     }
+}
+
+/*
+ * do_arg_all(): Open up to "count" windows, one for each argument.
+ */
+    static void
+do_arg_all(
+    int	count,
+    int	forceit,		// hide buffers in current windows
+    int keep_tabs)		// keep current tabs, for ":tab drop file"
+{
+    arg_all_state_T	aall;
+    win_T		*last_curwin;
+    tabpage_T		*last_curtab;
+    int			prev_arglist_locked = arglist_locked;
+
+#ifdef FEAT_CMDWIN
+    if (cmdwin_type != 0)
+    {
+	emsg(_(e_invalid_in_cmdline_window));
+	return;
+    }
+#endif
+    if (ARGCOUNT <= 0)
+    {
+	// Don't give an error message.  We don't want it when the ":all"
+	// command is in the .vimrc.
+	return;
+    }
+    setpcmark();
+
+    aall.use_firstwin = FALSE;
+    aall.had_tab = cmdmod.cmod_tab;
+    aall.new_curwin = NULL;
+    aall.new_curtab = NULL;
+    aall.forceit = forceit;
+    aall.keep_tabs = keep_tabs;
+    aall.opened_len = ARGCOUNT;
+    aall.opened = alloc_clear(aall.opened_len);
+    if (aall.opened == NULL)
+	return;
+
+    // Autocommands may do anything to the argument list.  Make sure it's not
+    // freed while we are working here by "locking" it.  We still have to
+    // watch out for its size being changed.
+    aall.alist = curwin->w_alist;
+    ++aall.alist->al_refcount;
+    arglist_locked = TRUE;
+
+#ifdef FEAT_GUI
+    need_mouse_correct = TRUE;
+#endif
+
+    // Try closing all windows that are not in the argument list.
+    // Also close windows that are not full width;
+    // When 'hidden' or "forceit" set the buffer becomes hidden.
+    // Windows that have a changed buffer and can't be hidden won't be closed.
+    // When the ":tab" modifier was used do this for all tab pages.
+    arg_all_close_unused_windows(&aall);
+
+    // Open a window for files in the argument list that don't have one.
+    // ARGCOUNT may change while doing this, because of autocommands.
+    if (count > aall.opened_len || count <= 0)
+	count = aall.opened_len;
+
+    // Don't execute Win/Buf Enter/Leave autocommands here.
+    ++autocmd_no_enter;
+    ++autocmd_no_leave;
+    last_curwin = curwin;
+    last_curtab = curtab;
+    win_enter(lastwin, FALSE);
+
+    /*
+     * Open upto "count" windows.
+     */
+    arg_all_open_windows(&aall, count);
 
     // Remove the "lock" on the argument list.
-    alist_unlink(alist);
+    alist_unlink(aall.alist);
     arglist_locked = prev_arglist_locked;
 
     --autocmd_no_enter;
 
     // restore last referenced tabpage's curwin
-    if (last_curtab != new_curtab)
+    if (last_curtab != aall.new_curtab)
     {
 	if (valid_tabpage(last_curtab))
 	    goto_tabpage_tp(last_curtab, TRUE, TRUE);
@@ -1208,13 +1260,13 @@ do_arg_all(
 	    win_enter(last_curwin, FALSE);
     }
     // to window with first arg
-    if (valid_tabpage(new_curtab))
-	goto_tabpage_tp(new_curtab, TRUE, TRUE);
-    if (win_valid(new_curwin))
-	win_enter(new_curwin, FALSE);
+    if (valid_tabpage(aall.new_curtab))
+	goto_tabpage_tp(aall.new_curtab, TRUE, TRUE);
+    if (win_valid(aall.new_curwin))
+	win_enter(aall.new_curwin, FALSE);
 
     --autocmd_no_leave;
-    vim_free(opened);
+    vim_free(aall.opened);
 }
 
 /*