internal/lsp: show critical error pop-ups as progress reports
We've been looking for a way to show unintrusive error status reports to
users--try using progress reports as a way of populating a status bar.
This avoids the problem of annoying the user with constant pop-ups.
Whenever an error is returns from (*snapshot).WorkspacePackages, we
start a progress report with the error message. If the error goes away
on the next call to diagnostics, or the error message changes, we will
either remove or update the progress report.
Screencast: https://drive.google.com/file/d/1tG9pc_tPsLoqkQHiJqdY8b06TzTkPVae/view?usp=sharing&resourcekey=0-CEG_LhGHYiFp9S37ut_kgw
Updates golang/go#42250
Change-Id: I8a529a911883092bc08af32246719d883dc5f5a2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/268677
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
diff --git a/gopls/internal/regtest/diagnostics_test.go b/gopls/internal/regtest/diagnostics_test.go
index a19394e..e3bd93f 100644
--- a/gopls/internal/regtest/diagnostics_test.go
+++ b/gopls/internal/regtest/diagnostics_test.go
@@ -1452,3 +1452,35 @@
)
})
}
+
+// TestProgressBarErrors confirms that critical workspace load errors are shown
+// and updated via progress reports.
+func TestProgressBarErrors(t *testing.T) {
+ testenv.NeedsGo1Point(t, 14)
+
+ const pkg = `
+-- go.mod --
+modul mod.com
+
+go 1.12
+-- main.go --
+package main
+`
+ run(t, pkg, func(t *testing.T, env *Env) {
+ env.OpenFile("go.mod")
+ env.Await(
+ OutstandingWork("Error loading workspace", "unknown directive"),
+ )
+ env.EditBuffer("go.mod", fake.NewEdit(0, 0, 3, 0, `module mod.com
+
+go 1.hello
+`))
+ env.Await(
+ OutstandingWork("Error loading workspace", "invalid go version"),
+ )
+ env.RegexpReplace("go.mod", "go 1.hello", "go 1.12")
+ env.Await(
+ NoOutstandingWork(),
+ )
+ })
+}
diff --git a/gopls/internal/regtest/env.go b/gopls/internal/regtest/env.go
index f1b958d..70859fd 100644
--- a/gopls/internal/regtest/env.go
+++ b/gopls/internal/regtest/env.go
@@ -58,8 +58,8 @@
}
type workProgress struct {
- title string
- percent float64
+ title, msg string
+ percent float64
}
func (s State) String() string {
@@ -200,10 +200,16 @@
switch kind := v["kind"]; kind {
case "begin":
work.title = v["title"].(string)
+ if msg, ok := v["message"]; ok {
+ work.msg = msg.(string)
+ }
case "report":
if pct, ok := v["percentage"]; ok {
work.percent = pct.(float64)
}
+ if msg, ok := v["message"]; ok {
+ work.msg = msg.(string)
+ }
case "end":
title := e.state.outstandingWork[m.Token].title
e.state.completedWork[title] = e.state.completedWork[title] + 1
diff --git a/gopls/internal/regtest/expectation.go b/gopls/internal/regtest/expectation.go
index efc5ffb..f895e8c 100644
--- a/gopls/internal/regtest/expectation.go
+++ b/gopls/internal/regtest/expectation.go
@@ -202,6 +202,24 @@
}
}
+// OutstandingWork expects a work item to be outstanding. The given title must
+// be an exact match, whereas the given msg must only be contained in the work
+// item's message.
+func OutstandingWork(title, msg string) SimpleExpectation {
+ check := func(s State) Verdict {
+ for _, work := range s.outstandingWork {
+ if work.title == title && strings.Contains(work.msg, msg) {
+ return Met
+ }
+ }
+ return Unmet
+ }
+ return SimpleExpectation{
+ check: check,
+ description: fmt.Sprintf("outstanding work: %s", title),
+ }
+}
+
// LogExpectation is an expectation on the log messages received by the editor
// from gopls.
type LogExpectation struct {
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index c54bdaa..f821361 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -182,34 +182,26 @@
// Diagnose all of the packages in the workspace.
wsPkgs, err := snapshot.WorkspacePackages(ctx)
+ if s.shouldIgnoreError(ctx, snapshot, err) {
+ return nil, nil
+ }
+ // Show the error as a progress error report so that it appears in the
+ // status bar. If a client doesn't support progress reports, the error
+ // will still be shown as a ShowMessage. If there is no error, any running
+ // error progress reports will be closed.
+ s.showCriticalErrorStatus(ctx, err)
+
if err != nil {
- if errors.Is(err, context.Canceled) {
- return nil, nil
- }
- // Some error messages can be displayed as diagnostics.
+ // Some error messages can also be displayed as diagnostics.
if errList := (*source.ErrorList)(nil); errors.As(err, &errList) {
if err := errorsToDiagnostic(ctx, snapshot, *errList, reports); err == nil {
return reports, nil
}
}
- // Try constructing a more helpful error message out of this error.
- if s.handleFatalErrors(ctx, snapshot, modErr, err) {
- return nil, nil
- }
event.Error(ctx, "errors diagnosing workspace", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
- // Present any `go list` errors directly to the user.
- if errors.Is(err, source.PackagesLoadError) {
- if err := s.client.ShowMessage(ctx, &protocol.ShowMessageParams{
- Type: protocol.Error,
- Message: fmt.Sprintf(`The code in the workspace failed to compile (see the error message below).
-If you believe this is a mistake, please file an issue: https://github.com/golang/go/issues/new.
-%v`, err),
- }); err != nil {
- event.Error(ctx, "ShowMessage failed", err, tag.Directory.Of(snapshot.View().Folder().Filename()))
- }
- }
return nil, nil
}
+
var (
showMsgMu sync.Mutex
showMsg *protocol.ShowMessageParams
@@ -299,6 +291,36 @@
return reports, showMsg
}
+// showCriticalErrorStatus shows the error as a progress report.
+// If the error is nil, it clears any existing error progress report.
+func (s *Server) showCriticalErrorStatus(ctx context.Context, err error) {
+ s.criticalErrorStatusMu.Lock()
+ defer s.criticalErrorStatusMu.Unlock()
+
+ // Remove all newlines so that the error message can be formatted in a
+ // status bar.
+ var errMsg string
+ if err != nil {
+ errMsg = strings.Replace(err.Error(), "\n", " ", -1)
+ }
+
+ if s.criticalErrorStatus == nil {
+ if errMsg != "" {
+ s.criticalErrorStatus = s.progress.start(ctx, "Error loading workspace", errMsg, nil, nil)
+ }
+ return
+ }
+
+ // If an error is already shown to the user, update it or mark it as
+ // resolved.
+ if errMsg == "" {
+ s.criticalErrorStatus.end("Done.")
+ s.criticalErrorStatus = nil
+ } else {
+ s.criticalErrorStatus.report(errMsg, 0)
+ }
+}
+
// checkForOrphanedFile checks that the given URIs can be mapped to packages.
// If they cannot and the workspace is not otherwise unloaded, it also surfaces
// a warning, suggesting that the user check the file for build tags.
@@ -470,7 +492,13 @@
return reports
}
-func (s *Server) handleFatalErrors(ctx context.Context, snapshot source.Snapshot, modErr, loadErr error) bool {
+func (s *Server) shouldIgnoreError(ctx context.Context, snapshot source.Snapshot, err error) bool {
+ if err == nil { // if there is no error at all
+ return false
+ }
+ if errors.Is(err, context.Canceled) {
+ return true
+ }
// If the folder has no Go code in it, we shouldn't spam the user with a warning.
var hasGo bool
_ = filepath.Walk(snapshot.View().Folder().Filename(), func(path string, info os.FileInfo, err error) error {
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index c2f5004..905a408 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -96,13 +96,19 @@
gcOptimizationDetailsMu sync.Mutex
gcOptimizationDetails map[span.URI]struct{}
- // diagnosticsSema limits the concurrency of diagnostics runs, which can be expensive.
+ // diagnosticsSema limits the concurrency of diagnostics runs, which can be
+ // expensive.
diagnosticsSema chan struct{}
progress *progressTracker
// debouncer is used for debouncing diagnostics.
debouncer *debouncer
+
+ // When the workspace fails to load, we show its status through a progress
+ // report with an error message.
+ criticalErrorStatusMu sync.Mutex
+ criticalErrorStatus *workDone
}
// sentDiagnostics is used to cache diagnostics that have been sent for a given file.
