Skip to content

Commit

Permalink
internal/lsp: add an importShortcut configuration
Browse files Browse the repository at this point in the history
This configuration deals with the fact that VS Code has the same
shortcut for go-to-definition and clicking a link. Many users don't like
the behavior of this shortcut triggering both requests, so we allow
users to choose between each behavior.

Fixes golang/go#39065

Change-Id: I760c99c1fade4d157515d3b0fd647daf0c8314eb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242457
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
  • Loading branch information
stamblerre committed Jul 15, 2020
1 parent 4cea897 commit ddb87c8
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 25 deletions.
45 changes: 24 additions & 21 deletions internal/lsp/link.go
Expand Up @@ -130,28 +130,31 @@ func goLinks(ctx context.Context, view source.View, fh source.FileHandle) ([]pro
return true
})
var links []protocol.DocumentLink
for _, imp := range imports {
// For import specs, provide a link to a documentation website, like https://pkg.go.dev.
target, err := strconv.Unquote(imp.Path.Value)
if err != nil {
continue
}
// See golang/go#36998: don't link to modules matching GOPRIVATE.
if view.IsGoPrivatePath(target) {
continue
}
if mod, version, ok := moduleAtVersion(ctx, target, ph); ok && strings.ToLower(view.Options().LinkTarget) == "pkg.go.dev" {
target = strings.Replace(target, mod, mod+"@"+version, 1)
}
// Account for the quotation marks in the positions.
start := imp.Path.Pos() + 1
end := imp.Path.End() - 1
target = fmt.Sprintf("https://%s/%s", view.Options().LinkTarget, target)
l, err := toProtocolLink(view, m, target, start, end, source.Go)
if err != nil {
return nil, err
// For import specs, provide a link to a documentation website, like
// https://pkg.go.dev.
if view.Options().ImportShortcut.ShowLinks() {
for _, imp := range imports {
target, err := strconv.Unquote(imp.Path.Value)
if err != nil {
continue
}
// See golang/go#36998: don't link to modules matching GOPRIVATE.
if view.IsGoPrivatePath(target) {
continue
}
if mod, version, ok := moduleAtVersion(ctx, target, ph); ok && strings.ToLower(view.Options().LinkTarget) == "pkg.go.dev" {
target = strings.Replace(target, mod, mod+"@"+version, 1)
}
// Account for the quotation marks in the positions.
start := imp.Path.Pos() + 1
end := imp.Path.End() - 1
target = fmt.Sprintf("https://%s/%s", view.Options().LinkTarget, target)
l, err := toProtocolLink(view, m, target, start, end, source.Go)
if err != nil {
return nil, err
}
links = append(links, l)
}
links = append(links, l)
}
for _, s := range str {
l, err := findLinksInString(ctx, view, s.Value, s.Pos(), m, source.Go)
Expand Down
9 changes: 6 additions & 3 deletions internal/lsp/source/identifier.go
Expand Up @@ -73,9 +73,12 @@ func Identifier(ctx context.Context, snapshot Snapshot, fh FileHandle, pos proto
var ErrNoIdentFound = errors.New("no identifier found")

func findIdentifier(ctx context.Context, s Snapshot, pkg Package, file *ast.File, pos token.Pos) (*IdentifierInfo, error) {
// Handle import specs separately, as there is no formal position for a package declaration.
if result, err := importSpec(s, pkg, file, pos); result != nil || err != nil {
return result, err
// Handle import specs separately, as there is no formal position for a
// package declaration.
if s.View().Options().ImportShortcut.ShowDefinition() {
if result, err := importSpec(s, pkg, file, pos); result != nil || err != nil {
return result, err
}
}
path := pathEnclosingObjNode(file, pos)
if path == nil {
Expand Down
35 changes: 34 additions & 1 deletion internal/lsp/source/options.go
Expand Up @@ -206,9 +206,14 @@ type UserOptions struct {
// StaticCheck enables additional analyses from staticcheck.io.
StaticCheck bool

// LinkTarget is the website used for documentation.
// LinkTarget is the website used for documentation. If empty, no link is
// provided.
LinkTarget string

// ImportShortcut specifies whether import statements should link to
// documentation or go to definitions. The default is both.
ImportShortcut ImportShortcut

// LocalPrefix is used to specify goimports's -local behavior.
LocalPrefix string

Expand Down Expand Up @@ -241,6 +246,22 @@ type UserOptions struct {
Gofumpt bool
}

type ImportShortcut int

const (
Both ImportShortcut = iota
Link
Definition
)

func (s ImportShortcut) ShowLinks() bool {
return s == Both || s == Link
}

func (s ImportShortcut) ShowDefinition() bool {
return s == Both || s == Definition
}

type completionOptions struct {
deepCompletion bool
unimported bool
Expand Down Expand Up @@ -505,6 +526,18 @@ func (o *Options) set(name string, value interface{}) OptionResult {
case "linkTarget":
result.setString(&o.LinkTarget)

case "importShortcut":
var s string
result.setString(&s)
switch s {
case "both":
o.ImportShortcut = Both
case "link":
o.ImportShortcut = Link
case "definition":
o.ImportShortcut = Definition
}

case "analyses":
result.setBoolMap(&o.UserEnabledAnalyses)

Expand Down

0 comments on commit ddb87c8

Please sign in to comment.