Closed newhook closed 8 years ago
newhook
commented
8 years ago I took a quick look at this and its not clear how best to get the information. godoc doesn't seem appropriate as I don't think it is super easy to get the package information from the current symbol. Anyone have any ideas?
lukehoban
commented
8 years ago The bits and pieces for this are available in the various Go tools, but I'm not sure there's one tool that does everything needed here yet.
The go-outline utility is a simple wrapper over go/ast to enumerate the top level definitions in a source file, including the start and end positions of each definition.
godef returns the file name and line number of the definition, so you could in principle pass that file into go-outline then do a search through the start/end pairs for one that encloses the definition position.
One downside is that now hovering over identifiers requires two separate process invocations that must be executed serially. That may be acceptable.
Another alternative would be to find or create a replacement for godef which provides more of this information automatically. Generally I'm not super happy with godef as a dependency since there doesn't appear to be any active work on it. But I'm not sure there is any sufficiently good replacement.
newhook
commented
8 years ago godef also doesn't work correctly in some cases with vendored dependencies. Would you object to a fork + mod to godep to get the doc comment?
lukehoban
commented
8 years ago Not opposed to that. Ideally could contribute the fixes back to godef.
I do wonder whether there is an alternative to godef that uses the built-in go/ast, go/parser, etc. instead of the custom code in github.com/rogpeppe/godef. There's so much code in that repo that it's hard to trust that it's up to date with the language. I feel like https://github.com/rogpeppe/godef/blob/master/godef.go could be reimplemented targeting the built-in Go AST instead for a much more targetted implementation of definition search. But haven't had a chance to look into this more myself yet.
I've also been meaning to look at whether oracle describe serves the purpose here well enough to be a replacement.
newhook
commented
8 years ago I never looked at the impl, but now I have ... yeah questions abound! Perhaps that is why it doesn't work sometimes ;)
Gys
commented
8 years ago I do not know what info a popup in js shows. So I might be completely of track here. But why not use go doc <sym.method> ? That shows the definition PLUS docs.
srinathh
commented
8 years ago There's a new tool been released called gogetdoc which takes a file path and a byte position offset and returns relevant documentation. It uses go/types under the hood.
Can that be used for showing documentation?
lukehoban
commented
8 years ago @srinathh The gogetdoc tool looks like it could be a good fit - unfortunately it appears to depend on Go 1.6. I'm not sure how to gracefully handle that in the vscode-go prereqs given the current state of things.
srinathh
commented
8 years ago Hmm... any pointers for where I can start looking into in the vscode-go codebase to try out something with this? (JS is not my primary language but I've done some web frontend type stuff)
On Tue, Apr 5, 2016 at 12:09 AM Luke Hoban notifications@github.com wrote:
@srinathh https://github.com/srinathh The gogetdoc tool looks like it could be a good fit - unfortunately it appears to depend on Go 1.6. I'm not sure how to gracefully handle that in the vscode-go prereqs given the current state of things.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/Microsoft/vscode-go/issues/87#issuecomment-205441348
lukehoban
commented
8 years ago @srinathh You could take a look at integrating gogetdoc instead of the godoc usage in https://github.com/Microsoft/vscode-go/blob/master/src/goDeclaration.ts#L59. It would also need to be added as a dependency here (and in docs): https://github.com/Microsoft/vscode-go/blob/ebce6f5c3c8d7dda94294e9f46517bbb8608d5ab/src/goInstallTools.ts#L38.
However, I don't think it's okay yet to require users of vscode-go to have Go 1.6. So there would need to be graceful fallbacks to other options both at acquisition time and when using features powered by this.
That said, it might be okay to only include documentation in hover tips, completion lists and signature help when using Go1.6 though.
Gys
commented
8 years ago This message was created automatically by mail delivery software.
A message that you sent could not be delivered to one or more of its recipients. This is a temporary error. The following address(es) deferred:
greulen@gmail.com Domain reulen.nl has exceeded the max emails per hour (10/5 (200%)) allowed. Message will be reattempted later
------- This is a copy of the message, including all the headers. ------
Received: from mail-oi0-f51.google.com ([209.85.218.51]:33758)
by fyvie.footholds.net with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128)
(Exim 4.861)
(envelope-from <greulen+caf=gijs=reulen.nl@gmail.com>)
id 1anV3K-0002zF-SO
for gijs@reulen.nl; Tue, 05 Apr 2016 18:46:10 +0100
Received: by mail-oi0-f51.google.com with SMTP id w85so26933868oiw.0
for gijs@reulen.nl; Tue, 05 Apr 2016 10:45:50 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20130820;
h=x-gm-message-state:delivered-to:date:dkim-signature:from:reply-to
:to:cc:message-id:in-reply-to:references:subject:mime-version
:content-transfer-encoding:precedence:list-id:list-archive:list-post
:list-unsubscribe;
bh=A6CRi+s0L8dIje6s5ZrImDuaX5/URVpiZ+UL0+bfww0=;
b=bDaYWQJRg5VKNXG7EBtV3CIBLpDId0BpwFzlahCKAljnab70ojtC3Z0ZqpanKUBTb0
Yhjy/5kHBRpM7JiOIfAHMK4McOmvgXHxWzqEA1GfLAl9Vq4hB4G4XzDT/C9cj9Dc6LTr
VrAIwrnGQY0dVVnFGvVrjFLiQ9R9CjkkdUYfe576AY/aZHVCymStestuCKN8N+LtPjJH
DEjslxjW0wy8G4rGCuUCc6xEoqKBcOdTGBPokYDkY6Z6BymaaLAhG5PcqmCye1dzIRab
/rHMcANmLjR2QuFUyPeLbh9s2lpZXHGTIMEwQ8lrUsFTIRAUJIzLyrvFi2Yp1UMunHM5
fX7w==
X-Gm-Message-State: AD7BkJJTwSfjSlk6uTBLynReVjoCZmS0HYfmm4yGvgMddBL7PSGJxdZEBt6kL7j7ZJnjbjGjXe0bboUa5X+oBJtUTv/k2ug=
X-Received: by 10.202.206.4 with SMTP id e4mr9456441oig.88.1459878329770;
Tue, 05 Apr 2016 10:45:29 -0700 (PDT)
X-Forwarded-To: gijs@reulen.nl
X-Forwarded-For: greulen@gmail.com gijs@reulen.nl
Delivered-To: greulen@gmail.com
Received: by 10.157.13.20 with SMTP id 20csp507816oti;
Tue, 5 Apr 2016 10:45:28 -0700 (PDT)
X-Received: by 10.140.104.13 with SMTP id z13mr13336768qge.68.1459878328856;
Tue, 05 Apr 2016 10:45:28 -0700 (PDT)
Received: from github-smtp2b-ext-cp1-prd.iad.github.net (github-smtp2-ext3.iad.github.net. [192.30.252.194])
by mx.google.com with ESMTPS id z93si27049127qgd.106.2016.04.05.10.45.28
for greulen@gmail.com
(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
Tue, 05 Apr 2016 10:45:28 -0700 (PDT)
Received-SPF: pass (google.com: domain of noreply@github.com designates 192.30.252.194 as permitted sender) client-ip=192.30.252.194;
Authentication-Results: mx.google.com;
dkim=pass (test mode) header.i=@github.com;
spf=pass (google.com: domain of noreply@github.com designates 192.30.252.194 as permitted sender) smtp.mailfrom=noreply@github.com;
dmarc=pass (p=NONE dis=NONE) header.from=github.com
Date: Tue, 05 Apr 2016 10:45:28 -0700
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com;
s=pf2014; t=1459878328;
bh=A6CRi+s0L8dIje6s5ZrImDuaX5/URVpiZ+UL0+bfww0=;
h=From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID:
List-Archive:List-Post:List-Unsubscribe:From;
b=PJrZfFPnfdQSnrpkUELI04q0Fyb8ZT/+m4FBarckuyeIwrfoGyTSj+E8G3EpvxhLv
D/tLc+HQZqOLxaoD+MXio7btLqc7+OOkWfWLttp8Jl9CJ0l/JDpyMIVWMpvq6aBsG/
hmN0gEFe7brN8yagKyWrhez4i5oCZ8IpBk5/ZewY=
From: Luke Hoban notifications@github.com
Reply-To: Microsoft/vscode-go reply@reply.github.com
To: Microsoft/vscode-go vscode-go@noreply.github.com
Cc: Gys greulen@gmail.com
Message-ID: Microsoft/vscode-go/issues/87/205916088@github.com
In-Reply-To: Microsoft/vscode-go/issues/87@github.com
References: Microsoft/vscode-go/issues/87@github.com
Subject: Re: [Microsoft/vscode-go] Add godoc to hover provider (#87)
Mime-Version: 1.0
Content-Type: multipart/alternative;
boundary="--==_mimepart_5703f9b85e0f5_20383f99178d12c06948d7";
charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: lukehoban
X-GitHub-Recipient: Gys
X-GitHub-Reason: comment
List-ID: Microsoft/vscode-go
----==_mimepart_5703f9b85e0f5_20383f99178d12c06948d7 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit
@srinathh You could take a look at integrating gogetdoc instead of the godoc usage in https://github.com/Microsoft/vscode-go/blob/master/src/goDeclaration.ts#L59. It would also need to be added as a dependency here (and in docs): https://github.com/Microsoft/vscode-go/blob/ebce6f5c3c8d7dda94294e9f46517bbb8608d5ab/src/goInstallTools.ts#L38.
However, I don't think it's okay yet to require users of vscode-go to have Go 1.6. So there would need to be graceful fallbacks to other options both at acquisition time and when using features powered by this.
That said, it might be okay to only include documentation in hover tips, completion lists and signature help when using Go1.6 though.
You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/Microsoft/vscode-go/issues/87#issuecomment-205916088 ----==_mimepart_5703f9b85e0f5_20383f99178d12c06948d7 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit
@srinathh You could take a look at integrating gogetdoc instead of the godoc usage in https://github.com/Microsoft/vscode-go/blob/master/src/goDeclaration.ts#L59. It would also need to be added as a dependency here (and in docs): https://github.com/Microsoft/vscode-go/blob/ebce6f5c3c8d7dda94294e9f46517bbb8608d5ab/src/goInstallTools.ts#L38.
However, I don't think it's okay yet to require users of vscode-go to have Go 1.6. So there would need to be graceful fallbacks to other options both at acquisition time and when using features powered by this.
That said, it might be okay to only include documentation in hover tips, completion lists and signature help when using Go1.6 though.
—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
----==_mimepart_5703f9b85e0f5_20383f99178d12c06948d7--
zmb3
commented
8 years ago One way around the Go 1.6 requirement would be to bundle or fetch a prebuilt gogetdoc binary rather than using go get to download and build the source.
I'd be open to releasing binaries for the major platforms on Github if this would help.
nii236
commented
8 years ago I noticed that the export func docstring is available when you open a bracket when calling a function. What tool is this using?
abarisain
commented
8 years ago I tried to take a shot at this bug, starting with the suggestions here: https://github.com/Microsoft/vscode-go/issues/87#issuecomment-205916088
I easily got gogetdoc to replace godoc (while keeping godoc for < 1.6), but I don't understand why godoc should be replaced with gogetdoc? godoc seems to work correctly, and vscode-go doesn't have the same pitfalls described in https://zmb3.github.io/post/gogetdoc/ gogetdoc does not return the method line in the file, so godef is still required. The only thing gogetdoc seems to bring to the table, is that it does not return the full package doc, but only the func we want. Maybe this is why you wanted to replace godef: https://github.com/Microsoft/vscode-go/issues/87#issuecomment-159598791 ? The comment is from 2015, and I haven't ran into the issue myself.
From what I understand, this bug is about the hover provider. It seems to disable the godoc call, and even if I force it to run, the provider doesn't show it. Is it disabled because it has to serially call two programs? To me, it looks like replacing godef/godoc is another issue that this one, which is for the hover popup.
I'm willing to get my hands dirty and submit a PR for this issue (or both!), but I'm not sure what the right approach is :)
abarisain
commented
8 years ago That said, I think that with the recent gofmt improvments, this issue is solved by some simple change: https://github.com/abarisain/vscode-go/commit/847be4807cfbc224de9e9b27327c26336ab01954 . It works pretty well and seems fast enough for continuous usage.
I'll probably send a PR soon describing some caveats I noticed
srinathh
commented
8 years ago wow this looks great! any way I can help beta test?
abarisain
commented
8 years ago Sure, but I'd rather not pollute this bug with that. Shoot me an email :)
zmb3
commented
8 years ago gogetdoc does not return the method line in the file, so godef is still required
I don't understand what you mean here. The purpose of gogetdoc is to free the editor developer from having to figure out what arguments to pass to godoc. You just give gogetdoc a file and the position of the cursor (or mouse in this case) and it finds the doc for you.
Godef is in no way required in order to use gogetdoc.
I believe gogetdoc to be a superior solution. It handles several difficult corner cases and is most likely faster than a godef + godoc solution (a single process is launched rather than two, the source is read once instead of twice, etc.)
I'm obviously biased, but I love vscode and I'd love to see it added to the list of editors powered by gogetdoc.
abarisain
commented
8 years ago Don't get me wrong, I'd like to integrate gogetdoc into vscode! I'm merely trying to figure out which direction to go.
In my early tests, it turned out to be slower, but I think there's something wrong with my machine so I'm not blaming anything yet.
The plugin does rely on godef not only to get the method definition, but also to get the line number and column so vscode can "jump to definition": https://github.com/Microsoft/vscode-go/blob/master/src/goDeclaration.ts#L50-L53 . While gogetdoc can get the documentation easily, we would still need to call godef to get this information, and do it in the same pass in which we get the documentation. So most of the benefits brought by gogetdoc are lost (and it even introduces the caveat of requiring Go 1.6). I don't know what's your opinion on adding that information to gogetdoc. It might be out of scope for your projet.
Anyway, that's a little out of scope for that bug in my opinion. The hover doc is implemented by the change I linked. Replacing godoc and godef is another issue, that I'd be happy to work on, especially if you're there to make sure I'm not using gogetdoc wrong :)
zmb3
commented
8 years ago Ah - I understand now.
Adding the filename and line number of the definition wouldn't be too difficult, I have that information already in order to get the doc. Maybe one of these days we can throw something together and see how it performs.
abarisain
commented
8 years ago That would be amazing. I guess we can open another issue here about switching to gogetdoc
nii236
commented
8 years ago Hooray, when is the next release coming so we can use this great feature? 👍
lukehoban
commented
8 years ago Just pushed an 0.6.43 release which includes this change.
nii236
commented
8 years ago YTMND!
zmb3
commented
8 years ago So now that this has been released and I see the behavior I can clarify what gogetdoc would bring to the table. It looks like the current implementation only provides doc for things in the standard library. The gogetdoc tool would also provide doc for code within your project, or any other code in your GOPATH. It also provides doc for builtins (len, append, cap, etc) and even package doc for import paths.
@abarisain if you're still interested in working together on this feel free to ping me (email in profile). Great work by the way - this is really nice 👍
lukehoban
commented
8 years ago @zmb3 I'd love to see a PR for integrating gogetdoc if you or anyone else is interested. There are two limitations with the currently merged feature (1) only works for standard library (2) only works for functions. It would be great to expand on both of those.
zmb3
commented
8 years ago Arnaud and I are working on it. Stay tuned :-)
On Saturday, August 20, 2016, Luke Hoban notifications@github.com wrote:
@zmb3 https://github.com/zmb3 I'd love to see a PR for integrating gogetdoc if you or anyone else is interested. There are two limitations with the currently merged feature (1) only works for standard library (2) only works for functions. It would be great to expand on both of those.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Microsoft/vscode-go/issues/87#issuecomment-241232479, or mute the thread https://github.com/notifications/unsubscribe-auth/AHLav-g4IopTSm-MGUWdJGrc5EHQm33Uks5qh6HXgaJpZM4Go6Ux .
I noticed in the javascript editor hovering over a function shows a hover popout with the function documentation. It would be nice to add support for this.