From c02aa52d2b72044787b2082c9844d129fe6d4d15 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Fri, 22 Nov 2019 00:52:09 -0500 Subject: [PATCH] internal/lsp: handle first change behavior on the server side I'm not sure why this was being managed by the view, but delete the code that handles tracking a file's first change. It is only used to avoid spamming the user with error messages. Change-Id: Id95089478ffb7e189d38cbc147e3dde6a1c55c5e Reviewed-on: https://go-review.googlesource.com/c/tools/+/208274 Reviewed-by: Ian Cottrell Reviewed-by: Heschi Kreinick Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- internal/lsp/cache/session.go | 37 +++++++++++----------------- internal/lsp/cache/view.go | 12 ++++----- internal/lsp/server.go | 4 +++ internal/lsp/source/view.go | 4 +-- internal/lsp/text_synchronization.go | 22 ++++++++++------- 5 files changed, 39 insertions(+), 40 deletions(-) diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index bbbcd9ccc2..81f350b967 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -51,9 +51,6 @@ type overlay struct { // sameContentOnDisk is true if a file has been saved on disk, // and therefore does not need to be part of the overlay sent to go/packages. sameContentOnDisk bool - - // unchanged is true if a file has not yet been edited. - unchanged bool } func (s *session) Options() source.Options { @@ -342,7 +339,7 @@ func (s *session) GetFile(uri span.URI, kind source.FileKind) source.FileHandle return s.cache.GetFile(uri, kind) } -func (s *session) SetOverlay(uri span.URI, kind source.FileKind, version float64, data []byte) bool { +func (s *session) SetOverlay(uri span.URI, kind source.FileKind, version float64, data []byte) { s.overlayMu.Lock() defer func() { s.overlayMu.Unlock() @@ -351,22 +348,17 @@ func (s *session) SetOverlay(uri span.URI, kind source.FileKind, version float64 if data == nil { delete(s.overlays, uri) - return false + return } - o := s.overlays[uri] - firstChange := o != nil && o.unchanged - s.overlays[uri] = &overlay{ - session: s, - uri: uri, - kind: kind, - data: data, - hash: hashContents(data), - version: version, - unchanged: o == nil, + session: s, + uri: uri, + kind: kind, + data: data, + hash: hashContents(data), + version: version, } - return firstChange } func (s *session) clearOverlay(uri span.URI) { @@ -385,13 +377,12 @@ func (s *session) openOverlay(ctx context.Context, uri span.URI, kind source.Fil s.filesWatchMap.Notify(uri, source.Open) }() s.overlays[uri] = &overlay{ - session: s, - uri: uri, - kind: kind, - data: data, - hash: hashContents(data), - unchanged: true, - version: version, + session: s, + uri: uri, + kind: kind, + data: data, + hash: hashContents(data), + version: version, } // If the file is on disk, check if its content is the same as the overlay. if _, hash, err := s.cache.GetFile(uri, kind).Read(ctx); err == nil { diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index dfd239f4f1..c8d5f91d13 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -324,21 +324,21 @@ func (v *view) getSnapshot() *snapshot { } // SetContent sets the overlay contents for a file. -func (v *view) SetContent(ctx context.Context, uri span.URI, version float64, content []byte) (bool, error) { +func (v *view) SetContent(ctx context.Context, uri span.URI, version float64, content []byte) { v.mu.Lock() defer v.mu.Unlock() + if v.Ignore(uri) { + return + } + // Cancel all still-running previous requests, since they would be // operating on stale data. v.cancel() v.backgroundCtx, v.cancel = context.WithCancel(v.baseCtx) - if v.Ignore(uri) { - return false, nil - } - kind := source.DetectLanguage("", uri.Filename()) - return v.session.SetOverlay(uri, kind, version, content), nil + v.session.SetOverlay(uri, kind, version, content) } // FindFile returns the file if the given URI is already a part of the view. diff --git a/internal/lsp/server.go b/internal/lsp/server.go index dacfd48b50..147061ebdb 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -14,6 +14,7 @@ import ( "golang.org/x/tools/internal/jsonrpc2" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/span" ) // NewClientServer @@ -83,6 +84,9 @@ type Server struct { undeliveredMu sync.Mutex undelivered map[source.FileIdentity][]source.Diagnostic + // changedFiles tracks files for which there has been a textDocument/didChange. + changedFiles map[span.URI]struct{} + // folders is only valid between initialize and initialized, and holds the // set of folders to build views for when we are ready pendingFolders []protocol.WorkspaceFolder diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 0a1ea13a79..1d0ab14f69 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -96,7 +96,7 @@ type View interface { FindFile(ctx context.Context, uri span.URI) File // Called to set the effective contents of a file from this view. - SetContent(ctx context.Context, uri span.URI, version float64, content []byte) (wasFirstChange bool, err error) + SetContent(ctx context.Context, uri span.URI, version float64, content []byte) // BackgroundContext returns a context used for all background processing // on behalf of this view. @@ -172,7 +172,7 @@ type Session interface { IsOpen(uri span.URI) bool // Called to set the effective contents of a file from this session. - SetOverlay(uri span.URI, kind FileKind, version float64, data []byte) (wasFirstChange bool) + SetOverlay(uri span.URI, kind FileKind, version float64, data []byte) // DidChangeOutOfBand is called when a file under the root folder changes. // If the file was open in the editor, it returns true. diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index c821a2e403..a979cca546 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -71,14 +71,10 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo if err != nil { return err } - wasFirstChange, err := view.SetContent(ctx, uri, params.TextDocument.Version, []byte(text)) - if err != nil { - return err - } - - // TODO: Ideally, we should be able to specify that a generated file should be opened as read-only. + view.SetContent(ctx, uri, params.TextDocument.Version, []byte(text)) + // Ideally, we should be able to specify that a generated file should be opened as read-only. // Tell the user that they should not be editing a generated file. - if source.IsGenerated(ctx, view, uri) && wasFirstChange { + if s.wasFirstChange(uri) && source.IsGenerated(ctx, view, uri) { s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ Message: fmt.Sprintf("Do not edit this file! %s is a generated file.", uri.Filename()), Type: protocol.Warning, @@ -91,6 +87,14 @@ func (s *Server) didChange(ctx context.Context, params *protocol.DidChangeTextDo return nil } +func (s *Server) wasFirstChange(uri span.URI) bool { + if s.changedFiles == nil { + s.changedFiles = make(map[span.URI]struct{}) + } + _, ok := s.changedFiles[uri] + return ok +} + func fullChange(changes []protocol.TextDocumentContentChangeEvent) (string, bool) { if len(changes) > 1 { return "", false @@ -151,6 +155,6 @@ func (s *Server) didClose(ctx context.Context, params *protocol.DidCloseTextDocu if err != nil { return err } - _, err = view.SetContent(ctx, uri, -1, nil) - return err + view.SetContent(ctx, uri, -1, nil) + return nil }