vim-patch:9.0.0388: the do_arg_all() function is too long

Problem:    The do_arg_all() function is too long.
Solution:   Split the function in smaller parts. (Yegappan Lakshmanan,
            closes vim/vim#11062)
https://github.com/vim/vim/commit/8894761daf68220504932c8b3e75f59138cdb617

1 files changed, 158 insertions, 122 deletions

diff --git a/src/nvim/arglist.c b/src/nvim/arglist.c
index cddd22b1bf..ae174403a0 100644
--- a/src/nvim/arglist.c
+++ b/src/nvim/arglist.c
@@ -27,6 +27,25 @@
 #include "nvim/vim.h"
 #include "nvim/window.h"
 
+/// State used by the :all command to open all the files in the argument list in
+/// separate windows.
+typedef struct {
+  alist_T *alist;     ///< argument list to be used
+  int had_tab;
+  bool keep_tabs;
+  bool forceit;
+
+  bool use_firstwin;  ///< use first window for arglist
+  uint8_t *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[]
+  win_T *new_curwin;
+  tabpage_T *new_curtab;
+} arg_all_state_T;
+
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "arglist.c.generated.h"
 #endif
@@ -501,17 +520,20 @@ void ex_args(exarg_T *eap)
     }
   }
 
+  // ":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);
-  } else if (eap->cmdidx == CMD_args) {
-    // ":args": list arguments.
+    return;
+  }
+
+  // ":args": list arguments.
+  if (eap->cmdidx == CMD_args) {
     if (ARGCOUNT <= 0) {
-      return;
+      return;  // empty argument list
     }
 
     char **items = xmalloc(sizeof(char *) * (size_t)ARGCOUNT);
@@ -525,10 +547,14 @@ void ex_args(exarg_T *eap)
     }
     list_in_columns(items, ARGCOUNT, curwin->w_arg_idx);
     xfree(items);
-  } else if (eap->cmdidx == CMD_arglocal) {
+
+    return;
+  }
+
+  // ":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.
     ga_grow(gap, GARGCOUNT);
 
     for (int i = 0; i < GARGCOUNT; i++) {
@@ -793,82 +819,32 @@ char *alist_name(aentry_T *aep)
   return bp->b_fname;
 }
 
-/// do_arg_all(): Open up to 'count' windows, one for each argument.
-///
-/// @param forceit  hide buffers in current windows
-/// @param keep_tabs  keep current tabs, for ":tab drop file"
-static void do_arg_all(int count, int forceit, int keep_tabs)
+/// 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)
 {
-  uint8_t *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
-  bool tab_drop_empty_window = false;
-  int split_ret = OK;
-  bool 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;
-  bool prev_arglist_locked = arglist_locked;
+  win_T *old_curwin = curwin;
+  tabpage_T *old_curtab = curtab;
 
-  assert(firstwin != NULL);  // satisfy coverity
-
-  if (cmdwin_type != 0) {
-    emsg(_(e_cmdwin));
-    return;
-  }
-  if (ARGCOUNT <= 0) {
-    // Don't give an error message.  We don't want it when the ":all" command is in the .vimrc.
-    return;
-  }
-  setpcmark();
-
-  opened_len = ARGCOUNT;
-  opened = xcalloc((size_t)opened_len, 1);
-
-  // 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;
-
-  old_curwin = curwin;
-  old_curtab = curtab;
-
-  // 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 (;;) {
     win_T *wpnext = NULL;
-    tpnext = curtab->tp_next;
+    tabpage_T *tpnext = curtab->tp_next;
     for (win_T *wp = firstwin; wp != NULL; wp = wpnext) {
       int i;
       wpnext = wp->w_next;
-      buf = wp->w_buffer;
+      buf_T *buf = wp->w_buffer;
       if (buf->b_ffname == NULL
-          || (!keep_tabs && (buf->b_nwindows > 1 || wp->w_width != Columns))) {
-        i = opened_len;
+          || (!aall->keep_tabs && (buf->b_nwindows > 1 || wp->w_width != Columns))) {
+        i = aall->opened_len;
       } else {
         // check if the buffer in this window is in the arglist
-        for (i = 0; i < opened_len; i++) {
-          if (i < alist->al_ga.ga_len
-              && (AARGLIST(alist)[i].ae_fnum == buf->b_fnum
-                  || path_full_compare(alist_name(&AARGLIST(alist)[i]),
+        for (i = 0; i < aall->opened_len; i++) {
+          if (i < aall->alist->al_ga.ga_len
+              && (AARGLIST(aall->alist)[i].ae_fnum == buf->b_fnum
+                  || path_full_compare(alist_name(&AARGLIST(aall->alist)[i]),
                                        buf->b_ffname,
                                        true, true) & kEqualFiles)) {
             int weight = 1;
@@ -880,23 +856,24 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
               }
             }
 
-            if (weight > (int)opened[i]) {
-              opened[i] = (uint8_t)weight;
+            if (weight > (int)aall->opened[i]) {
+              aall->opened[i] = (uint8_t)weight;
               if (i == 0) {
-                if (new_curwin != NULL) {
-                  new_curwin->w_arg_idx = opened_len;
+                if (aall->new_curwin != NULL) {
+                  aall->new_curwin->w_arg_idx = aall->opened_len;
                 }
-                new_curwin = wp;
-                new_curtab = curtab;
+                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) {
-              // Use the current argument list for all windows containing a file from it.
+            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;
@@ -905,8 +882,8 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
       }
       wp->w_arg_idx = i;
 
-      if (i == opened_len && !keep_tabs) {    // close this window
-        if (buf_hide(buf) || forceit || buf->b_nwindows > 1
+      if (i == aall->opened_len && !aall->keep_tabs) {  // close this window
+        if (buf_hide(buf) || aall->forceit || buf->b_nwindows > 1
             || !bufIsChanged(buf)) {
           // If the buffer was changed, and we would like to hide it, try autowriting.
           if (!buf_hide(buf) && buf->b_nwindows <= 1 && bufIsChanged(buf)) {
@@ -921,8 +898,8 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
           }
           // 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), false);
             // check if autocommands removed the next window
@@ -936,7 +913,7 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
     }
 
     // Without the ":tab" modifier only do the current tab page.
-    if (had_tab == 0 || tpnext == NULL) {
+    if (aall->had_tab == 0 || tpnext == NULL) {
       break;
     }
 
@@ -946,39 +923,35 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
     }
     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 up to "count" windows for the files in the argument list "aall->alist".
+static void arg_all_open_windows(arg_all_state_T *aall, int count)
+{
+  bool tab_drop_empty_window = false;
 
-  // 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 && buf_is_empty(curbuf) && curbuf->b_nwindows == 1
+  if (aall->keep_tabs && buf_is_empty(curbuf) && curbuf->b_nwindows == 1
       && curbuf->b_ffname == NULL && !curbuf->b_changed) {
-    use_firstwin = true;
+    aall->use_firstwin = true;
     tab_drop_empty_window = true;
   }
 
+  int split_ret = OK;
+
   for (int 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_IN_TAB(wp, curtab) {
           if (wp->w_arg_idx == i) {
-            if (keep_tabs) {
-              new_curwin = wp;
-              new_curtab = curtab;
+            if (aall->keep_tabs) {
+              aall->new_curwin = wp;
+              aall->new_curtab = curtab;
             } else if (wp->w_frame->fr_parent != curwin->w_frame->fr_parent) {
               emsg(_("E249: window layout changed unexpectedly"));
               i = count;
@@ -995,8 +968,8 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
       if (tab_drop_empty_window && i == count - 1) {
         autocmd_no_enter--;
       }
-      if (!use_firstwin) {              // split current window
-        p_ea_save = p_ea;
+      if (!aall->use_firstwin) {        // split current window
+        bool p_ea_save = p_ea;
         p_ea = true;                    // use space from all windows
         split_ret = win_split(0, WSP_ROOM | WSP_BELOW);
         p_ea = p_ea_save;
@@ -1010,38 +983,101 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
       // edit file "i"
       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,
+      (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,
+                      || 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;
     }
     os_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.
+///
+/// @param forceit    hide buffers in current windows
+/// @param keep_tabs  keep current tabs, for ":tab drop file"
+static void do_arg_all(int count, int forceit, int keep_tabs)
+{
+  win_T *last_curwin;
+  tabpage_T *last_curtab;
+  bool prev_arglist_locked = arglist_locked;
+
+  assert(firstwin != NULL);  // satisfy coverity
+
+  if (cmdwin_type != 0) {
+    emsg(_(e_cmdwin));
+    return;
+  }
+  if (ARGCOUNT <= 0) {
+    // Don't give an error message.  We don't want it when the ":all" command is in the .vimrc.
+    return;
+  }
+  setpcmark();
+
+  arg_all_state_T aall = {
+    .use_firstwin = false,
+    .had_tab = cmdmod.cmod_tab,
+    .new_curwin = NULL,
+    .new_curtab = NULL,
+    .forceit = forceit,
+    .keep_tabs = keep_tabs,
+    .opened_len = ARGCOUNT,
+    .opened = xcalloc((size_t)ARGCOUNT, 1),
+  };
+
+  // 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.
+  aall.alist = curwin->w_alist;
+  aall.alist->al_refcount++;
+  arglist_locked = true;
+
+  // 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);
     }
@@ -1050,15 +1086,15 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
     }
   }
   // to window with first arg
-  if (valid_tabpage(new_curtab)) {
-    goto_tabpage_tp(new_curtab, true, true);
+  if (valid_tabpage(aall.new_curtab)) {
+    goto_tabpage_tp(aall.new_curtab, true, true);
   }
-  if (win_valid(new_curwin)) {
-    win_enter(new_curwin, false);
+  if (win_valid(aall.new_curwin)) {
+    win_enter(aall.new_curwin, false);
   }
 
   autocmd_no_leave--;
-  xfree(opened);
+  xfree(aall.opened);
 }
 
 /// ":all" and ":sall".