vim-patch:8.0.1754: ex_helpgrep() is too long #11084

Problem:    ex_helpgrep() is too long.
Solution:   Refactor the function. (Yegappan Lakshmanan, closes vim/vim#2766)
2225ebb486

src/nvim/quickfix.c

@@ -5593,45 +5593,12 @@ void ex_cexpr(exarg_T *eap)
   }
 }
 
-/*
+// Get the location list for ":lhelpgrep"
- * ":helpgrep {pattern}"
+static qf_info_T *hgr_get_ll(bool *new_ll)
- */
+  FUNC_ATTR_NONNULL_ALL
-void ex_helpgrep(exarg_T *eap)
 {
-  regmatch_T regmatch;
+  win_T *wp;
-  char_u      *save_cpo;
+  qf_info_T *qi;
-  char_u      *p;
-  int fcount;
-  char_u      **fnames;
-  FILE        *fd;
-  int fi;
-  long lnum;
-  char_u      *lang;
-  qf_info_T   *qi = &ql_info;
-  int new_qi = FALSE;
-  char_u      *au_name =  NULL;
-
-  /* Check for a specified language */
-  lang = check_help_lang(eap->arg);
-
-  switch (eap->cmdidx) {
-  case CMD_helpgrep:  au_name = (char_u *)"helpgrep"; break;
-  case CMD_lhelpgrep: au_name = (char_u *)"lhelpgrep"; break;
-  default: break;
-  }
-  if (au_name != NULL && apply_autocmds(EVENT_QUICKFIXCMDPRE, au_name,
-                                        curbuf->b_fname, true, curbuf)) {
-    if (aborting()) {
-      return;
-    }
-  }
-
-  /* Make 'cpoptions' empty, the 'l' flag should not be used here. */
-  save_cpo = p_cpo;
-  p_cpo = empty_option;
-
-  if (eap->cmdidx == CMD_lhelpgrep) {
-    win_T *wp = NULL;
 
   // If the current window is a help window, then use it
   if (bt_help(curwin->w_buffer)) {
@@ -5647,70 +5614,55 @@ void ex_helpgrep(exarg_T *eap)
     qi = wp->w_llist;
   }
   if (qi == NULL) {
-      /* Allocate a new location list for help text matches */
+    // Allocate a new location list for help text matches
-      qi = ll_new_list();
+    if ((qi = ll_new_list()) == NULL) {
-      new_qi = TRUE;
+      return NULL;
     }
+    *new_ll = true;
   }
 
-  regmatch.regprog = vim_regcomp(eap->arg, RE_MAGIC + RE_STRING);
+  return qi;
-  regmatch.rm_ic = FALSE;
-  if (regmatch.regprog != NULL) {
-    // Create a new quickfix list.
-    qf_new_list(qi, qf_cmdtitle(*eap->cmdlinep));
-
-    // Go through all the directories in 'runtimepath'
-    p = p_rtp;
-    while (*p != NUL && !got_int) {
-      copy_option_part(&p, NameBuff, MAXPATHL, ",");
-
-      /* Find all "*.txt" and "*.??x" files in the "doc" directory. */
-      add_pathsep((char *)NameBuff);
-      STRCAT(NameBuff, "doc/*.\\(txt\\|??x\\)");
-
-      // Note: We cannot just do `&NameBuff` because it is a statically sized array
-      //       so `NameBuff == &NameBuff` according to C semantics.
-      char_u *buff_list[1] = {NameBuff};
-      if (gen_expand_wildcards(1, buff_list, &fcount,
-              &fnames, EW_FILE|EW_SILENT) == OK
-          && fcount > 0) {
-        for (fi = 0; fi < fcount && !got_int; fi++) {
-          // Skip files for a different language.
-          if (lang != NULL
-              && STRNICMP(lang, fnames[fi] + STRLEN(fnames[fi]) - 3, 2) != 0
-              && !(STRNICMP(lang, "en", 2) == 0
-                   && STRNICMP("txt", fnames[fi]
-                               + STRLEN(fnames[fi]) - 3, 3) == 0)) {
-            continue;
 }
-          fd = os_fopen((char *)fnames[fi], "r");
+
-          if (fd != NULL) {
+// Search for a pattern in a help file.
-            lnum = 1;
+static void hgr_search_file(
+    qf_info_T *qi,
+    char_u *fname,
+    regmatch_T *p_regmatch)
+  FUNC_ATTR_NONNULL_ARG(1, 3)
+{
+  FILE *const fd = os_fopen((char *)fname, "r");
+  if (fd == NULL) {
+    return;
+  }
+
+  long lnum = 1;
   while (!vim_fgets(IObuff, IOSIZE, fd) && !got_int) {
     char_u *line = IObuff;
-              if (vim_regexec(&regmatch, line, (colnr_T)0)) {
+
+    if (vim_regexec(p_regmatch, line, (colnr_T)0)) {
       int l = (int)STRLEN(line);
 
-                /* remove trailing CR, LF, spaces, etc. */
+      // remove trailing CR, LF, spaces, etc.
-                while (l > 0 && line[l - 1] <= ' ')
+      while (l > 0 && line[l - 1] <= ' ') {
         line[--l] = NUL;
+      }
 
       if (qf_add_entry(qi,
                        qi->qf_curlist,
                        NULL,   // dir
-                                 fnames[fi],
+                       fname,
                        NULL,
                        0,
                        line,
                        lnum,
-                                 (int)(regmatch.startp[0] - line)
+                       (int)(p_regmatch->startp[0] - line) + 1,  // col
-                                 + 1,                   // col
                        false,  // vis_col
                        NULL,   // search pattern
                        0,      // nr
                        1,      // type
-                                 true)                  // valid
+                       true    // valid
-                    == FAIL) {
+                       ) == FAIL) {
         got_int = true;
         if (line != IObuff) {
           xfree(line);
@@ -5718,18 +5670,106 @@ void ex_helpgrep(exarg_T *eap)
         break;
       }
     }
-              if (line != IObuff)
+    if (line != IObuff) {
       xfree(line);
-              ++lnum;
+    }
+    lnum++;
     line_breakcheck();
   }
   fclose(fd);
 }
+
+// Search for a pattern in all the help files in the doc directory under
+// the given directory.
+static void hgr_search_files_in_dir(
+    qf_info_T *qi,
+    char_u *dirname,
+    regmatch_T *p_regmatch,
+    const char_u *lang)
+  FUNC_ATTR_NONNULL_ARG(1, 2, 3)
+{
+  int fcount;
+  char_u **fnames;
+
+  // Find all "*.txt" and "*.??x" files in the "doc" directory.
+  add_pathsep((char *)dirname);
+  STRCAT(dirname, "doc/*.\\(txt\\|??x\\)");  // NOLINT
+  if (gen_expand_wildcards(1, &dirname, &fcount,
+                           &fnames, EW_FILE|EW_SILENT) == OK
+      && fcount > 0) {
+    for (int fi = 0; fi < fcount && !got_int; fi++) {
+      // Skip files for a different language.
+      if (lang != NULL
+          && STRNICMP(lang, fnames[fi] + STRLEN(fnames[fi]) - 3, 2) != 0
+          && !(STRNICMP(lang, "en", 2) == 0
+               && STRNICMP("txt", fnames[fi] + STRLEN(fnames[fi]) - 3, 3)
+               == 0)) {
+        continue;
+      }
+
+      hgr_search_file(qi, fnames[fi], p_regmatch);
     }
     FreeWild(fcount, fnames);
   }
 }
 
+// Search for a pattern in all the help files in the 'runtimepath'.
+static void hgr_search_in_rtp(qf_info_T *qi, regmatch_T *p_regmatch,
+                              char_u *arg)
+  FUNC_ATTR_NONNULL_ALL
+{
+  // Check for a specified language
+  char_u *const lang = check_help_lang(arg);
+
+  // Go through all directories in 'runtimepath'
+  char_u *p = p_rtp;
+  while (*p != NUL && !got_int) {
+    copy_option_part(&p, NameBuff, MAXPATHL, ",");
+
+    hgr_search_files_in_dir(qi, NameBuff, p_regmatch, lang);
+  }
+}
+
+// ":helpgrep {pattern}"
+void ex_helpgrep(exarg_T *eap)
+{
+  qf_info_T *qi = &ql_info;
+  bool new_qi = false;
+  char_u *au_name =  NULL;
+
+  switch (eap->cmdidx) {
+  case CMD_helpgrep:  au_name = (char_u *)"helpgrep"; break;
+  case CMD_lhelpgrep: au_name = (char_u *)"lhelpgrep"; break;
+  default: break;
+  }
+  if (au_name != NULL && apply_autocmds(EVENT_QUICKFIXCMDPRE, au_name,
+                                        curbuf->b_fname, true, curbuf)) {
+    if (aborting()) {
+      return;
+    }
+  }
+
+  // Make 'cpoptions' empty, the 'l' flag should not be used here.
+  char_u *const save_cpo = p_cpo;
+  p_cpo = empty_option;
+
+  if (eap->cmdidx == CMD_lhelpgrep) {
+    qi = hgr_get_ll(&new_qi);
+    if (qi == NULL) {
+      return;
+    }
+  }
+
+  regmatch_T regmatch = {
+    .regprog = vim_regcomp(eap->arg, RE_MAGIC + RE_STRING),
+    .rm_ic = false,
+  };
+  if (regmatch.regprog != NULL) {
+    // Create a new quickfix list.
+    qf_new_list(qi, qf_cmdtitle(*eap->cmdlinep));
+
+    hgr_search_in_rtp(qi, &regmatch, eap->arg);
+
     vim_regfree(regmatch.regprog);
 
     qi->qf_lists[qi->qf_curlist].qf_nonevalid = FALSE;

src/nvim/testdir/test_quickfix.vim

@@ -2317,6 +2317,12 @@ func XvimgrepTests(cchar)
   call assert_equal('Xtestfile2', bufname(''))
   call assert_equal('Editor:Emacs EmAcS', l[0].text)
 
+  " Test for unloading a buffer after vimgrep searched the buffer
+  %bwipe
+  Xvimgrep /Editor/j Xtestfile*
+  call assert_equal(0, getbufinfo('Xtestfile1')[0].loaded)
+  call assert_equal([], getbufinfo('Xtestfile2'))
+
   call delete('Xtestfile1')
   call delete('Xtestfile2')
 endfunc