lsp: Use filename path for current buffer for defns#1732
lsp: Use filename path for current buffer for defns#1732charlieegan3 merged 2 commits intoopen-policy-agent:mainfrom
Conversation
There was a bug here where the defn would only work when the result was another file. Now we use the same format of reference for the current buffer as we use for other modules (a path rather than a URI) Fixes open-policy-agent#1700 Signed-off-by: Charlie Egan <charlie_egan@apple.com>
There was a problem hiding this comment.
Thank you! This not working has been a real pain in the neck, so this is a much welcome fix <3 Perhaps worth breaking out the Filename and URI value creation just so that we can test them? And make sure we can't change them accidentally without also having to change tests (which likely will alert us about breaking something)?
(even if it means some pretty silly tests)
|
The tests are a pretty similar to the existing URI tests, but I've added some new helpers for the relative operations. We're doing these in a few other places too, if mostly in tests. Lmk if that look ok. |
|
One thing I dislike is the client identifier functionality. I think I perhaps added that prematurely when after noticing some minor differences in VSCode. I think my preference is to keep it for now, and to do a review of client specific behaviour separately. |
| query := oracle.DefinitionQuery{ | ||
| // The value of Filename is used if the defn in the current buffer. | ||
| Filename: params.TextDocument.URI, | ||
| Filename: uri.ToRelativePath(l.client.Identifier, params.TextDocument.URI, l.workspaceRootURI), |
There was a problem hiding this comment.
There's also
func (l *LanguageServer) toRelativePath(fileURI string) string {
return strings.TrimPrefix(l.toPath(fileURI), l.workspacePath()+string(os.PathSeparator))
}Which could be updated to use your new function, and then we can use that here instead
There was a problem hiding this comment.
Ahh! I missed that one...
There was a problem hiding this comment.
OK, updated that function now.
Signed-off-by: Charlie Egan <charlie_egan@apple.com>
charlieegan3
force-pushed
the
uri-defn
branch
from
5c600b6 to
dedd92b
Compare
2 participants
There was a bug here where the defn would only work when the result was another file. Now we use the same format of reference for the current buffer as we use for other modules (a path rather than a URI)
Fixes #1700