From 1058ed41f41a753118f11138e463285d488ceee9 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Thu, 4 Apr 2019 19:33:08 -0400 Subject: [PATCH] internal/lsp: add a "usePlaceholders" setting to gopls configuration This change allows us to determine if we should complete with placeholders in the function signature or not. We currently complete all functions with parameters with a set of parentheses with the cursor between them. Now, we support the option to tab through the parameters. Change-Id: I01d9d7ffdc76eb2f8e009ab9c8aa46d71c24c358 Reviewed-on: https://go-review.googlesource.com/c/tools/+/170877 TryBot-Result: Gobot Gobot Reviewed-by: Ian Cottrell --- internal/lsp/completion.go | 65 ++++++++++++++--------- internal/lsp/server.go | 102 ++++++++++++++++--------------------- 2 files changed, 86 insertions(+), 81 deletions(-) diff --git a/internal/lsp/completion.go b/internal/lsp/completion.go index c2c8cda8..1ea91183 100644 --- a/internal/lsp/completion.go +++ b/internal/lsp/completion.go @@ -6,15 +6,42 @@ package lsp import ( "bytes" + "context" "fmt" "sort" "strings" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/span" ) -func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string, pos protocol.Position, snippetsSupported, signatureHelpEnabled bool) []protocol.CompletionItem { +func (s *Server) completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) { + uri := span.NewURI(params.TextDocument.URI) + view := s.findView(ctx, uri) + f, m, err := newColumnMap(ctx, view, uri) + if err != nil { + return nil, err + } + spn, err := m.PointSpan(params.Position) + if err != nil { + return nil, err + } + rng, err := spn.Range(m.Converter) + if err != nil { + return nil, err + } + items, prefix, err := source.Completion(ctx, f, rng.Start) + if err != nil { + return nil, err + } + return &protocol.CompletionList{ + IsIncomplete: false, + Items: toProtocolCompletionItems(items, prefix, params.Position, s.snippetsSupported, s.usePlaceholders), + }, nil +} + +func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string, pos protocol.Position, snippetsSupported, usePlaceholders bool) []protocol.CompletionItem { insertTextFormat := protocol.PlainTextTextFormat if snippetsSupported { insertTextFormat = protocol.SnippetTextFormat @@ -24,14 +51,11 @@ func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string }) items := []protocol.CompletionItem{} for i, candidate := range candidates { - // Matching against the label. + // Match against the label. if !strings.HasPrefix(candidate.Label, prefix) { continue } - // InsertText is deprecated in favor of TextEdits. - // TODO(rstambler): Remove this logic when we are confident that we no - // longer need to support it. - insertText, triggerSignatureHelp := labelToProtocolSnippets(candidate.Label, candidate.Kind, insertTextFormat, signatureHelpEnabled) + insertText := labelToInsertText(candidate.Label, candidate.Kind, insertTextFormat, usePlaceholders) if strings.HasPrefix(insertText, prefix) { insertText = insertText[len(prefix):] } @@ -54,12 +78,6 @@ func toProtocolCompletionItems(candidates []source.CompletionItem, prefix string FilterText: insertText, Preselect: i == 0, } - // If we are completing a function, we should trigger signature help if possible. - if triggerSignatureHelp && signatureHelpEnabled { - item.Command = &protocol.Command{ - Command: "editor.action.triggerParameterHints", - } - } items = append(items, item) } return items @@ -90,13 +108,13 @@ func toProtocolCompletionItemKind(kind source.CompletionItemKind) protocol.Compl } } -func labelToProtocolSnippets(label string, kind source.CompletionItemKind, insertTextFormat protocol.InsertTextFormat, signatureHelpEnabled bool) (string, bool) { +func labelToInsertText(label string, kind source.CompletionItemKind, insertTextFormat protocol.InsertTextFormat, usePlaceholders bool) string { switch kind { case source.ConstantCompletionItem: // The label for constants is of the format " = ". // We should not insert the " = " part of the label. if i := strings.Index(label, " ="); i >= 0 { - return label[:i], false + return label[:i] } case source.FunctionCompletionItem, source.MethodCompletionItem: var trimmed, params string @@ -105,16 +123,16 @@ func labelToProtocolSnippets(label string, kind source.CompletionItemKind, inser params = strings.Trim(label[i:], "()") } if params == "" || trimmed == "" { - return label, true + return label } // Don't add parameters or parens for the plaintext insert format. if insertTextFormat == protocol.PlainTextTextFormat { - return trimmed, true + return trimmed } - // If we do have signature help enabled, the user can see parameters as - // they type in the function, so we just return empty parentheses. - if signatureHelpEnabled { - return trimmed + "($1)", true + // If we don't want to use placeholders, just add 2 parentheses with + // the cursor in the middle. + if !usePlaceholders { + return trimmed + "($1)" } // If signature help is not enabled, we should give the user parameters // that they can tab through. The insert text format follows the @@ -131,11 +149,12 @@ func labelToProtocolSnippets(label string, kind source.CompletionItemKind, inser if i != 0 { b.WriteString(", ") } - fmt.Fprintf(b, "${%v:%v}", i+1, r.Replace(strings.Trim(p, " "))) + p = strings.Split(strings.Trim(p, " "), " ")[0] + fmt.Fprintf(b, "${%v:%v}", i+1, r.Replace(p)) } b.WriteByte(')') - return b.String(), false + return b.String() } - return label, false + return label } diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 01b252e3..851b94a7 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -75,7 +75,9 @@ type Server struct { initializedMu sync.Mutex initialized bool // set once the server has received "initialize" request - signatureHelpEnabled bool + // Configurations. + // TODO(rstambler): Separate these into their own struct? + usePlaceholders bool snippetsSupported bool configurationSupported bool dynamicConfigurationSupported bool @@ -102,36 +104,14 @@ func (s *Server) Initialize(ctx context.Context, params *protocol.InitializePara } s.initialized = true // mark server as initialized now - // Check if the client supports snippets in completion items. - if x, ok := params.Capabilities["textDocument"].(map[string]interface{}); ok { - if x, ok := x["completion"].(map[string]interface{}); ok { - if x, ok := x["completionItem"].(map[string]interface{}); ok { - if x, ok := x["snippetSupport"].(bool); ok { - s.snippetsSupported = x - } - } - } - } - // Check if the client supports configuration messages. - if x, ok := params.Capabilities["workspace"].(map[string]interface{}); ok { - if x, ok := x["configuration"].(bool); ok { - s.configurationSupported = x - } - if x, ok := x["didChangeConfiguration"].(map[string]interface{}); ok { - if x, ok := x["dynamicRegistration"].(bool); ok { - s.dynamicConfigurationSupported = x - } - } - } - - s.signatureHelpEnabled = true - // TODO(rstambler): Change this default to protocol.Incremental (or add a // flag). Disabled for now to simplify debugging. s.textDocumentSyncKind = protocol.Full - //We need a "detached" context so it does not get timeout cancelled. - //TODO(iancottrell): Do we need to copy any values across? + s.setClientCapabilities(params.Capabilities) + + // We need a "detached" context so it does not get timeout cancelled. + // TODO(iancottrell): Do we need to copy any values across? viewContext := context.Background() folders := params.WorkspaceFolders if len(folders) == 0 { @@ -195,6 +175,30 @@ func (s *Server) Initialize(ctx context.Context, params *protocol.InitializePara }, nil } +func (s *Server) setClientCapabilities(caps protocol.ClientCapabilities) { + // Check if the client supports snippets in completion items. + if x, ok := caps["textDocument"].(map[string]interface{}); ok { + if x, ok := x["completion"].(map[string]interface{}); ok { + if x, ok := x["completionItem"].(map[string]interface{}); ok { + if x, ok := x["snippetSupport"].(bool); ok { + s.snippetsSupported = x + } + } + } + } + // Check if the client supports configuration messages. + if x, ok := caps["workspace"].(map[string]interface{}); ok { + if x, ok := x["configuration"].(bool); ok { + s.configurationSupported = x + } + if x, ok := x["didChangeConfiguration"].(map[string]interface{}); ok { + if x, ok := x["dynamicRegistration"].(bool); ok { + s.dynamicConfigurationSupported = x + } + } + } +} + func (s *Server) Initialized(ctx context.Context, params *protocol.InitializedParams) error { if s.configurationSupported { if s.dynamicConfigurationSupported { @@ -346,28 +350,7 @@ func (s *Server) DidClose(ctx context.Context, params *protocol.DidCloseTextDocu } func (s *Server) Completion(ctx context.Context, params *protocol.CompletionParams) (*protocol.CompletionList, error) { - uri := span.NewURI(params.TextDocument.URI) - view := s.findView(ctx, uri) - f, m, err := newColumnMap(ctx, view, uri) - if err != nil { - return nil, err - } - spn, err := m.PointSpan(params.Position) - if err != nil { - return nil, err - } - rng, err := spn.Range(m.Converter) - if err != nil { - return nil, err - } - items, prefix, err := source.Completion(ctx, f, rng.Start) - if err != nil { - return nil, err - } - return &protocol.CompletionList{ - IsIncomplete: false, - Items: toProtocolCompletionItems(items, prefix, params.Position, s.snippetsSupported, s.signatureHelpEnabled), - }, nil + return s.completion(ctx, params) } func (s *Server) CompletionResolve(context.Context, *protocol.CompletionItem) (*protocol.CompletionItem, error) { @@ -640,16 +623,19 @@ func (s *Server) processConfig(view *cache.View, config interface{}) error { if !ok { return fmt.Errorf("invalid config gopls type %T", config) } - env := c["env"] - if env == nil { - return nil + // Get the environment for the go/packages config. + if env := c["env"]; env != nil { + menv, ok := env.(map[string]interface{}) + if !ok { + return fmt.Errorf("invalid config gopls.env type %T", env) + } + for k, v := range menv { + view.Config.Env = applyEnv(view.Config.Env, k, v) + } } - menv, ok := env.(map[string]interface{}) - if !ok { - return fmt.Errorf("invalid config gopls.env type %T", env) - } - for k, v := range menv { - view.Config.Env = applyEnv(view.Config.Env, k, v) + // Check if placeholders are enabled. + if usePlaceholders, ok := c["usePlaceholders"].(bool); ok { + s.usePlaceholders = usePlaceholders } return nil }