internal/lsp: only show command errors once

We both showing errors as a popup and returning them to the client,
which would then most likely show them as a popup. Don't return errors
back to the client.

Change-Id: I8486534e19399d514752900025d6db8db85d5086
Reviewed-on: https://go-review.googlesource.com/c/tools/+/272090
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 7fc954b..a8a26aa 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -63,9 +63,9 @@
 			source.CommandRemoveDependency.ID(),
 			source.CommandVendor.ID():
 			// TODO(PJW): for Toggle, not an error if it is being disabled
-			err := errors.New("all files must be saved first")
+			err := errors.New("All files must be saved first")
 			s.showCommandError(ctx, command.Title, err)
-			return nil, err
+			return nil, nil
 		}
 	}
 	ctx, cancel := context.WithCancel(xcontext.Detach(ctx))
@@ -79,15 +79,31 @@
 		// convenient for assertions.
 		work = s.progress.start(ctx, command.Title, "Running...", params.WorkDoneToken, cancel)
 	}
-	if command.Async {
-		go func() {
-			defer cancel()
-			s.runCommand(ctx, work, command, params.Arguments)
-		}()
-		return nil, nil
+
+	run := func() {
+		defer cancel()
+		err := s.runCommand(ctx, work, command, params.Arguments)
+		switch {
+		case errors.Is(err, context.Canceled):
+			work.end(command.Title + ": canceled")
+		case err != nil:
+			event.Error(ctx, fmt.Sprintf("%s: command error", command.Title), err)
+			work.end(command.Title + ": failed")
+			// Show a message when work completes with error, because the progress end
+			// message is typically dismissed immediately by LSP clients.
+			s.showCommandError(ctx, command.Title, err)
+		default:
+			work.end(command.ID() + ": completed")
+		}
 	}
-	defer cancel()
-	return nil, s.runCommand(ctx, work, command, params.Arguments)
+	if command.Async {
+		go run()
+	} else {
+		run()
+	}
+	// Errors running the command are displayed to the user above, so don't
+	// return them.
+	return nil, nil
 }
 
 func (s *Server) runSuggestedFixCommand(ctx context.Context, command *source.Command, args []json.RawMessage) error {
@@ -131,20 +147,6 @@
 }
 
 func (s *Server) runCommand(ctx context.Context, work *workDone, command *source.Command, args []json.RawMessage) (err error) {
-	defer func() {
-		switch {
-		case errors.Is(err, context.Canceled):
-			work.end(command.Title + ": canceled")
-		case err != nil:
-			event.Error(ctx, fmt.Sprintf("%s: command error", command.Title), err)
-			work.end(command.Title + ": failed")
-			// Show a message when work completes with error, because the progress end
-			// message is typically dismissed immediately by LSP clients.
-			s.showCommandError(ctx, command.Title, err)
-		default:
-			work.end(command.ID() + ": completed")
-		}
-	}()
 	// If the command has a suggested fix function available, use it and apply
 	// the edits to the workspace.
 	if command.IsSuggestedFix() {