This repository has been archived by the owner on Oct 12, 2022. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update gocode #305
Update gocode #305
Changes from 6 commits
cad92cb
bbd3c49
8305a24
57c86bd
e54ccd0
9c95515
68907e0
3d6ca62
622d511
e8fb6d4
8104bff
7b8431e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.