Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Update gocode #305

Merged
merged 12 commits into from
Sep 17, 2018
Merged
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
language: go

go:
- 1.8.x
- 1.9.x
- 1.10.x
anjmao marked this conversation as resolved.
Show resolved Hide resolved
- 1.11.x

go_import_path: github.com/sourcegraph/go-langserver

Expand Down
21 changes: 16 additions & 5 deletions langserver/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"

"github.com/sourcegraph/go-langserver/langserver/internal/gocode"
"github.com/sourcegraph/go-langserver/langserver/internal/gocode/gbimporter"
"github.com/sourcegraph/go-langserver/langserver/util"
"github.com/sourcegraph/go-langserver/pkg/lsp"
"github.com/sourcegraph/jsonrpc2"
Expand Down Expand Up @@ -46,11 +47,21 @@ func (h *LangHandler) handleTextDocumentCompletion(ctx context.Context, conn jso
return nil, fmt.Errorf("invalid position: %s:%d:%d (%s)", filename, params.Position.Line, params.Position.Character, why)
}

ca, rangelen := gocode.AutoComplete(contents, filename, offset)
citems := make([]lsp.CompletionItem, len(ca))
for i, it := range ca {
ac, err := gocode.AutoComplete(&gocode.AutoCompleteRequest{
Filename: filename,
Data: contents,
Cursor: offset,
Builtin: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keegancsmith I was missing this flag, after adding Builtin: true I was able to fix failing tests except whos with package autocomplete. There is one bug in gocode , if I place dot inside package path I get all builtins. In real world this is not a big deal and this can also be fixed with package autocomplete.

image

Source: !h.config.UseBinaryPkgCache,
Context: gbimporter.PackContext(h.BuildContext(ctx)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keegancsmith Thanks for review.

As I'm not using gocode daemon (not starting any net listener) I'm setting build context here. In current master https://github.com/sourcegraph/go-langserver/blob/master/langserver/internal/gocode/export.go#L17 is used only in unit tests.
There are failing integration tests since this gocode fork returns different suggestions for some cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to update those tests instead of just commenting them out?

Ok great about setting the buildcontext where you set it, that actually makes a lot of sense.

})
if err != nil {
return nil, fmt.Errorf("could not autocomplete %s: %v", filename, err)
}
citems := make([]lsp.CompletionItem, len(ac.Candidates))
for i, it := range ac.Candidates {
var kind lsp.CompletionItemKind
switch it.Class.String() {
switch it.Class {
case "const":
kind = CIKConstantSupported
case "func":
Expand All @@ -75,7 +86,7 @@ func (h *LangHandler) handleTextDocumentCompletion(ctx context.Context, conn jso
InsertText: newText,
TextEdit: &lsp.TextEdit{
Range: lsp.Range{
Start: lsp.Position{Line: params.Position.Line, Character: params.Position.Character - rangelen},
Start: lsp.Position{Line: params.Position.Line, Character: params.Position.Character - ac.Len},
End: lsp.Position{Line: params.Position.Line, Character: params.Position.Character},
},
NewText: newText,
Expand Down
4 changes: 0 additions & 4 deletions langserver/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
opentracing "github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"

"github.com/sourcegraph/go-langserver/langserver/internal/gocode"
"github.com/sourcegraph/go-langserver/pkg/lsp"
"github.com/sourcegraph/go-langserver/pkg/lspext"
"github.com/sourcegraph/jsonrpc2"
Expand Down Expand Up @@ -222,9 +221,6 @@ func (h *LangHandler) Handle(ctx context.Context, conn jsonrpc2.JSONRPC2, req *j
if err := h.reset(&params); err != nil {
return nil, err
}
if h.config.GocodeCompletionEnabled {
gocode.InitDaemon(h.BuildContext(ctx))
}

// PERF: Kick off a workspace/symbol in the background to warm up the server
if yes, _ := strconv.ParseBool(envWarmupOnInitialize); yes {
Expand Down
Loading