cmd/govim: do not add short-lived buffers during :vimgrep (#1034)

Using ":vimgrep" could result in a lot of short-lived buffers that is
sent to gopls since each file searched will trigger BufReadPost
autocommands.

Use QuickFixCmdPre & QuickFixCmdPost to detect when vimgrep runs,
and only add new buffers that still exists when vimgrep is done.
(see https://groups.google.com/g/vim_dev/c/qpu2O8cvprY/m/ZGAaOOlJBAAJ
for additional context)

Updates #1028

cmd/govim/buffer_events.go

@@ -18,7 +18,19 @@ import (
 
 func (v *vimstate) bufReadPost(args ...json.RawMessage) error {
 	nb := v.currentBufferInfo(args[0])
+	if v.vimgrepPendingBufs != nil {
+		// We are getting BufRead autocommands during a vimgrep.
 
+		// Save the buffer without adding it yet since vimgrep could open
+		// a lot of files that are closed immediately, and we only want to
+		// add buffers still open when vimgrep is done.
+		v.vimgrepPendingBufs[nb.Num] = nb
+		return nil
+	}
+	return v.addBuffer(nb)
+}
+
+func (v *vimstate) addBuffer(nb *types.Buffer) error {
 	// If we load a buffer that already had diagnostics reported by gopls, the buffer number must be
 	// updated to ensure that sign placement etc. works.
 	diags := *v.diagnosticsCache
@@ -65,6 +77,29 @@ func (v *vimstate) bufReadPost(args ...json.RawMessage) error {
 	return v.handleBufferEvent(nb)
 }
 
+func (v *vimstate) bufQuickFixCmdPre(args ...json.RawMessage) error {
+	v.vimgrepPendingBufs = make(map[int]*types.Buffer)
+	return nil
+}
+
+func (v *vimstate) bufQuickFixCmdPost(args ...json.RawMessage) error {
+	defer func() { v.vimgrepPendingBufs = nil }()
+
+	v.quickfixIsDiagnostics = v.quickfixIsDiagnostics && len(v.vimgrepPendingBufs) == 0
+
+	// Vim versions older than 8.2.2185 did not notify when buffers opened during vimgrep
+	// closed so we need to explicitly check if any of the buffers are still open.
+	for _, b := range v.vimgrepPendingBufs {
+		if v.ParseInt(v.ChannelCall("bufexists", b.Num)) != 1 {
+			continue
+		}
+		if err := v.addBuffer(b); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 type bufChangedChange struct {
 	Lnum  uint32   `json:"lnum"`
 	Col   uint32   `json:"col"`
@@ -186,6 +221,10 @@ func (v *vimstate) deleteCurrentBuffer(args ...json.RawMessage) error {
 		return fmt.Errorf("tried to remove buffer %v; but we have no record of it", currBufNr)
 	}
 
+	if v.vimgrepPendingBufs != nil {
+		delete(v.vimgrepPendingBufs, currBufNr)
+	}
+
 	// The diagnosticsCache is updated with -1 (unknown buffer) as bufnr.
 	// We don't want to remove the entries completely here since we want to show them in
 	// the quickfix window. And we don't need to remove existing signs or text properties

cmd/govim/main.go

@@ -31,11 +31,9 @@ const (
 	PluginPrefix = "GOVIM"
 )
 
-var (
-	// exposeTestAPI is a rather hacky but clean way of only exposing certain
-	// functions, commands and autocommands to Vim when run from a test
-	exposeTestAPI = os.Getenv(testsetup.EnvLoadTestAPI) == "true"
-)
+// exposeTestAPI is a rather hacky but clean way of only exposing certain
+// functions, commands and autocommands to Vim when run from a test
+var exposeTestAPI = os.Getenv(testsetup.EnvLoadTestAPI) == "true"
 
 func mainerr() error {
 	if err := flagSet.Parse(os.Args[1:]); err != nil {
@@ -288,6 +286,8 @@ func (g *govimplugin) Init(gg govim.Govim, errCh chan error) error {
 	g.DefineAutoCommand("", govim.Events{govim.EventBufRead, govim.EventBufNewFile}, govim.Patterns{"*.go", "go.mod", "go.sum"}, false, g.vimstate.bufReadPost, exprAutocmdCurrBufInfo)
 	g.DefineAutoCommand("", govim.Events{govim.EventBufWritePre}, govim.Patterns{"*.go", "go.mod", "go.sum"}, false, g.vimstate.formatCurrentBuffer, "eval(expand('<abuf>'))")
 	g.DefineAutoCommand("", govim.Events{govim.EventBufWritePost}, govim.Patterns{"*.go", "go.mod", "go.sum"}, false, g.vimstate.bufWritePost, "eval(expand('<abuf>'))")
+	g.DefineAutoCommand("", govim.Events{govim.EventQuickFixCmdPre}, govim.Patterns{"*vimgrep*"}, false, g.vimstate.bufQuickFixCmdPre)
+	g.DefineAutoCommand("", govim.Events{govim.EventQuickFixCmdPost}, govim.Patterns{"*vimgrep*"}, false, g.vimstate.bufQuickFixCmdPost)
 	g.DefineFunction(string(config.FunctionComplete), []string{"findarg", "base"}, g.vimstate.complete)
 	g.DefineCommand(string(config.CommandGoToDef), g.vimstate.gotoDef, govim.NArgsZeroOrOne)
 	g.DefineCommand(string(config.CommandGoToTypeDef), g.vimstate.gotoTypeDef, govim.NArgsZeroOrOne)

cmd/govim/vimstate.go

@@ -89,6 +89,17 @@ type vimstate struct {
 	// lastProgressText points to the text in the most recently created progress
 	// popup.
 	lastProgressText *strings.Builder
+
+	// vimgrepPendingBufs contain buffers read during a vimgrep quickfix command,
+	// keyed by buffer number.
+	// The purpose is to avoid sending DidOpen/DidClose notifications to gopls
+	// for all files which contained no pattern match.
+	// Vim will reuse the buffer number as long as the searched file didn't contain
+	// a pattern match. See discussion at https://groups.google.com/g/vim_dev/c/qpu2O8cvprY.
+	//
+	// A nil value is used to indicate that there is no ongoing vimgrep.
+	// When vimgrep is done, these buffers are added to govim.
+	vimgrepPendingBufs map[int]*types.Buffer
 }
 
 func (v *vimstate) setConfig(args ...json.RawMessage) (interface{}, error) {