From adb45749da8e536f7cc9ab862ecba1bd0235c192 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 29 Aug 2019 02:43:58 -0400 Subject: [PATCH] internal/lsp: turn on completion documentation by default This feature has been in an experimental state for a long enough time that I think we can enable it by default at master. Change-Id: I9bbb8b41377719f0e97f608f6e5163a883a176b3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/192259 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/completion.go | 2 +- internal/lsp/general.go | 3 + internal/lsp/lsp_test.go | 2 +- internal/lsp/source/completion.go | 10 ++- internal/lsp/source/completion_format.go | 96 ++++++++++++------------ internal/lsp/source/source_test.go | 1 - 6 files changed, 59 insertions(+), 55 deletions(-) diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index 29a9b1e4b5..8d5ccffad7 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -26,7 +26,7 @@ func (s *Server) completion(ctx context.Context, params *protocol.CompletionPara candidates, surrounding, err := source.Completion(ctx, view, f, params.Position, source.CompletionOptions{ WantDeepCompletion: !s.disableDeepCompletion, WantFuzzyMatching: !s.disableFuzzyMatching, - WantDocumentaton: s.wantCompletionDocumentation, + NoDocumentation: !s.wantCompletionDocumentation, WantFullDocumentation: s.hoverKind == fullDocumentation, WantUnimported: s.wantUnimportedCompletions, }) diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 3dcbb85e8b..ddb760ae21 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -273,9 +273,12 @@ func (s *Server) processConfig(ctx context.Context, view source.View, config int } // Check if the user wants documentation in completion items. + // This defaults to true. + s.wantCompletionDocumentation = true if wantCompletionDocumentation, ok := c["wantCompletionDocumentation"].(bool); ok { s.wantCompletionDocumentation = wantCompletionDocumentation } + // Check if placeholders are enabled. if usePlaceholders, ok := c["usePlaceholders"].(bool); ok { s.usePlaceholders = usePlaceholders diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index ed7aa7cf09..746348bb64 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -110,9 +110,9 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests r.server.disableDeepCompletion = true r.server.disableFuzzyMatching = true r.server.wantUnimportedCompletions = false - r.server.wantCompletionDocumentation = false }() + // Set this as a default. r.server.wantCompletionDocumentation = true for src, itemList := range data { diff --git a/internal/lsp/source/completion.go b/internal/lsp/source/completion.go index 84e229ff04..3d571b5e7b 100644 --- a/internal/lsp/source/completion.go +++ b/internal/lsp/source/completion.go @@ -380,11 +380,13 @@ type candidate struct { } type CompletionOptions struct { - WantDeepCompletion bool - WantDocumentaton bool + WantDeepCompletion bool + WantFuzzyMatching bool + + WantUnimported bool + + NoDocumentation bool WantFullDocumentation bool - WantUnimported bool - WantFuzzyMatching bool } // Completion returns a list of possible candidates for completion, given a diff --git a/internal/lsp/source/completion_format.go b/internal/lsp/source/completion_format.go index d17678ad3c..bdf7166569 100644 --- a/internal/lsp/source/completion_format.go +++ b/internal/lsp/source/completion_format.go @@ -113,56 +113,56 @@ func (c *completer) item(cand candidate) (CompletionItem, error) { plainSnippet: plainSnippet, placeholderSnippet: placeholderSnippet, } - // TODO(rstambler): Log errors when this feature is enabled. - if c.opts.WantDocumentaton { - declRange, err := objToRange(c.ctx, c.view, obj) - if err != nil { - goto Return - } - pos := c.view.Session().Cache().FileSet().Position(declRange.spanRange.Start) - if !pos.IsValid() { - goto Return - } - uri := span.FileURI(pos.Filename) - f, err := c.view.GetFile(c.ctx, uri) - if err != nil { - goto Return - } - gof, ok := f.(GoFile) - if !ok { - goto Return - } - pkg, err := gof.GetCachedPackage(c.ctx) - if err != nil { - goto Return - } - var ph ParseGoHandle - for _, h := range pkg.GetHandles() { - if h.File().Identity().URI == gof.URI() { - ph = h - } - } - if ph == nil { - goto Return - } - file, _ := ph.Cached(c.ctx) - if file == nil { - goto Return - } - ident, err := findIdentifier(c.ctx, c.view, gof, pkg, file, declRange.spanRange.Start) - if err != nil { - goto Return - } - hover, err := ident.Hover(c.ctx) - if err != nil { - goto Return - } - item.Documentation = hover.Synopsis - if c.opts.WantFullDocumentation { - item.Documentation = hover.FullDocumentation + // If the user doesn't want documentation for completion items. + if c.opts.NoDocumentation { + return item, nil + } + declRange, err := objToRange(c.ctx, c.view, obj) + if err != nil { + return item, nil + } + pos := c.view.Session().Cache().FileSet().Position(declRange.spanRange.Start) + if !pos.IsValid() { + return item, nil + } + uri := span.FileURI(pos.Filename) + f, err := c.view.GetFile(c.ctx, uri) + if err != nil { + return item, nil + } + gof, ok := f.(GoFile) + if !ok { + return item, nil + } + pkg, err := gof.GetCachedPackage(c.ctx) + if err != nil { + return item, nil + } + var ph ParseGoHandle + for _, h := range pkg.GetHandles() { + if h.File().Identity().URI == gof.URI() { + ph = h } } -Return: + if ph == nil { + return item, nil + } + file, _ := ph.Cached(c.ctx) + if file == nil { + return item, nil + } + ident, err := findIdentifier(c.ctx, c.view, gof, pkg, file, declRange.spanRange.Start) + if err != nil { + return item, nil + } + hover, err := ident.Hover(c.ctx) + if err != nil { + return item, nil + } + item.Documentation = hover.Synopsis + if c.opts.WantFullDocumentation { + item.Documentation = hover.FullDocumentation + } return item, nil } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 75d8501135..a80b919356 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -104,7 +104,6 @@ func (r *runner) Completion(t *testing.T, data tests.Completions, snippets tests }, source.CompletionOptions{ WantDeepCompletion: deepComplete, WantFuzzyMatching: fuzzyMatch, - WantDocumentaton: true, WantUnimported: unimported, }) if err != nil {