Use gogetdoc instead of godef and godoc#622
Conversation
ramya-rao-a
force-pushed
the
gogetdoc-godoc
branch
from
5463682 to
755bd27
Compare
ramya-rao-a
force-pushed
the
gogetdoc-godoc
branch
from
755bd27 to
aa102a8
Compare
ramya-rao-a
force-pushed
the
gogetdoc-godoc
branch
4 times, most recently
from
32aa034 to
aeca9ac
Compare
|
In travis, the unit tests are failing for 1.7 but pass in 1.6 mac... which is baffling @leaxoy if you have some time to look at the test failures, I'd appreciate it |
test/go.test.ts
Outdated
| return vscode.workspace.openTextDocument(uri).then((textDocument) => { | ||
| let promises = testCases.map(([position, expectedSignature, expectedDocumentation]) => | ||
| provider.provideHover(textDocument, position, null).then(res => { | ||
| // TODO: Documentation appears to currently be broken on Go 1.7, so disabling these tests for now |
There was a problem hiding this comment.
Can we give reenabling these tests a shot? When this was commented, they worked on my machine
src/goSignature.ts
Outdated
| text = res.declarationlines[0].substring(5); | ||
| si = new SignatureInformation(text, res.doc); | ||
| let braceStart = text.indexOf('('); | ||
| sig = text.substring(braceStart); |
There was a problem hiding this comment.
Does that work for methods that have a signature like func (l *Struct) Add(a int, b int) {? I did it somewhat differently for that reason https://github.com/abarisain/vscode-go/blob/gogetdoc2/src/goSignature.ts#L33-L40
If it does, then sorry! Haven't had the time to test this branch yet.
There was a problem hiding this comment.
In such case it thinks the receiver is the first param, thats a bug.
There was a problem hiding this comment.
Fixed it, Took your code and tested. Will push the changes soon.
ramya-rao-a
force-pushed
the
gogetdoc-godoc
branch
2 times, most recently
from
d2127d3 to
d0c6671
Compare
|
All tests are a pass! |
ramya-rao-a
force-pushed
the
gogetdoc-godoc
branch
from
ac9beff to
73dbfc4
Compare
|
Added additional tests to cover various scenarios where using |
src/goInstallTools.ts
Outdated
| // Install the doc/def tool that was chosen by the user | ||
| if (goConfig['docsTool'] === 'godoc') { | ||
| tools['godef'] = 'github.com/rogpeppe/godef'; | ||
| } else if (goConfig['formatTool'] === 'goreturns') { |
There was a problem hiding this comment.
I think that should be } else if (goConfig['docsTool'] === 'gogetdoc') { :)
There was a problem hiding this comment.
Good catch, I should really stop copy pasting things...
|
LGTM :-) |
|
Ditto |
ramya-rao-a
force-pushed
the
gogetdoc-godoc
branch
from
1d16d00 to
8fb8dbc
Compare
|
Great addition to the plugin ! Congrats @zmb3 |
5 participants
Currently we use
godeffor Goto/Peek Definition.godef+godoctogether provide the Hover Info and Signature Help.These tools have shortcomings which we hope to overcome by using
gogetdocinstead.#440 #442 #515 #567 #459 #496
This PR is work on top of what @leaxoy has done in #607
What has been added are the below
godoc(old way of doing thingsgodef+godoc) vsgogetdocgogetdocwill solve. We will reach out to those users and ask them to change the setting to usegogetdocand give feedback