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

rootPath should be a path not URI #38

Closed
prabirshrestha opened this issue Nov 24, 2016 · 10 comments
Closed

rootPath should be a path not URI #38

prabirshrestha opened this issue Nov 24, 2016 · 10 comments
Assignees
Labels

Comments

@prabirshrestha
Copy link

What is the correct way to path format for windows? Seems like backslash and colon confuses go-langserver.

langserver-go: reading on stdin, writing on stdout
--> request #1: initialize: {"capabilities":{},"rootPath":"file:///C:/Users/prshrest/go/src/github.com/prabirshrestha/del256"}
<-- result #1: initialize: {"capabilities":{"textDocumentSync":1,"hoverProvider":true,"definitionProvider":true,"referencesProvider":true,"documentSymbolProvider":true,"workspaceSymbolProvider":true}}
--> request #2: textDocument/definition: {"textDocument":{"uri":"file:///C:/Users/prshrest/go/src/github.com/prabirshrestha/del256/del256.go"},"position":{"character":32,"line":13}}
panic serving textDocument/definition: bad uri "file:///C:/Users/prshrest/go/src/github.com/prabirshrestha/del256/del256.go" (path "/C:/Users/prshrest/go/src/github.com/prabirshrestha/del256/del256.go" MUST NOT contain ':')
goroutine 7 [running]:
github.com/sourcegraph/go-langserver/langserver.(*LangHandler).Handle.func1(0xc04202be08, 0xc04203de30)
	c:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/handler.go:99 +0x12f
panic(0x7dffc0, 0xc0420da900)
	C:/Go/src/runtime/panic.go:458 +0x251
github.com/sourcegraph/go-langserver/langserver.(*HandlerShared).FilePath(0xc042008d40, 0xc0420f20f0, 0x4b, 0xa37c60, 0xc0420e8f90)
	c:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/fs.go:83 +0x403
github.com/sourcegraph/go-langserver/langserver.(*LangHandler).typecheck(0xc0420d1420, 0xa37c60, 0xc0420e8f90, 0xa31ee0, 0xc0420d1490, 0xc0420f20f0, 0x4b, 0xd, 0x20, 0x0, ...)
	c:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/loader.go:33 +0x41a
github.com/sourcegraph/go-langserver/langserver.(*LangHandler).handleDefinition(0xc0420d1420, 0xa37c60, 0xc0420e8c60, 0xa31ee0, 0xc0420d1490, 0xc04203de30, 0xc0420f20f0, 0x4b, 0xd, 0x20, ...)
	c:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/definition.go:13 +0xaf
github.com/sourcegraph/go-langserver/langserver.(*LangHandler).Handle(0xc0420d1420, 0xa37c60, 0xc0420e8c60, 0xa31ee0, 0xc0420d1490, 0xc04203de30, 0x0, 0x0, 0xa31d60, 0xc0420da910)
	c:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/handler.go:193 +0x1d7b
github.com/sourcegraph/go-langserver/langserver.(*LangHandler).handle(0xc0420d1420, 0xa37be0, 0xc04200c2b0, 0xc0420d1490, 0xc04203de30, 0x0, 0x0, 0x0, 0x0)
	c:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/handler.go:84 +0x6d
github.com/sourcegraph/go-langserver/langserver.(*LangHandler).(github.com/sourcegraph/go-langserver/langserver.handle)-fm(0xa37be0, 0xc04200c2b0, 0xc0420d1490, 0xc04203de30, 0x0, 0x0, 0x0, 0x0)
	c:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/handler.go:25 +0x59
github.com/sourcegraph/jsonrpc2.HandlerWithError.Handle(0xc04200d8e0, 0xa37be0, 0xc04200c2b0, 0xc0420d1490, 0xc04203de30)
	c:/Users/prshrest/go/src/github.com/sourcegraph/jsonrpc2/handler_with_error.go:15 +0x76
created by github.com/sourcegraph/jsonrpc2.(*Conn).readMessages
	c:/Users/prshrest/go/src/github.com/sourcegraph/jsonrpc2/jsonrpc2.go:421 +0x4ed
<-- error #2: textDocument/definition: {"code":0,"message":"unexpected panic: bad uri \"file:///C:/Users/prshrest/go/src/github.com/prabirshrestha/del256/del256.go\" (path \"/C:/Users/prshrest/go/src/github.com/prabirshrestha/del256/del256.go\" MUST NOT contain ':')","data":null}
@keegancsmith keegancsmith self-assigned this Nov 24, 2016
@keegancsmith
Copy link
Member

This seems to be the upstream bug in Go, not sure if it is solved. golang/go#6027

I'm very unfamiliar with windows, but will take a look into this since the stack trace makes it look easy to write a unit test for.

@keegancsmith
Copy link
Member

Hey sorry for not updating. So we are not spec complaint here, we are expecting a URI for the path, but in this instance it should just be a filepath. I'll update. Related issue microsoft/language-server-protocol#113

@keegancsmith keegancsmith changed the title file uri for windows rootPath should be a path not URI Dec 6, 2016
@keegancsmith
Copy link
Member

Apologies for not fixing this sooner. It should be minor to fix, so will prioritize soon.

@keegancsmith
Copy link
Member

Once #71 and #73 land, this should work on windows. However, I do not have a windows machine handy to test, so will appreciate confirmation at that time.

@prabirshrestha
Copy link
Author

@keegancsmith thanks for taking your time to look into this issue.
If you do have a temp branch that contains both the PR fix I can try it on my windows machine to verify if it works or not.

@keegancsmith
Copy link
Member

@prabirshrestha master should contain both fixes now.

@prabirshrestha
Copy link
Author

prabirshrestha commented Jan 11, 2017

Seems better but here is the log.

langserver-go: reading on stdin, writing on stdout
--> request #1: initialize: {"rootUri":"file:///C:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver","capabilities":{},"rootPath":"file:///C:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver"}
<-- result #1: initialize: {"capabilities":{"textDocumentSync":1,"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"referencesProvider":true,"documentSymbolProvider":true,"workspaceSymbolProvider":true,"xworkspaceReferencesProvider":true,"xdefinitionProvider":true}}
--> request #2: textDocument/didOpen: {"textDocument":{"uri":"file:///C:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/ast.go","version":1,"languageId":"typescript","text":"package langserver\n\nimport (\n\t\"fmt\"\n\t\"go/ast\"\n\t\"go/token\"\n\n\t\"github.com/sourcegraph/go-langserver/pkg/lsp\"\n)\n\nfunc offsetForPosition(contents []byte, p lsp.Position) (offset int, valid bool, whyInvalid string) {\n\tline := 0\n\tcol := 0\n\t// TODO(sqs): count chars, not bytes, per LSP. does that mean we\n\t// need to maintain 2 separate counters since we still need to\n\t// return the offset as bytes?\n\tfor _, b := range contents {\n\t\tif line == p.Line \u0026\u0026 col == p.Character {\n\t\t\treturn offset, true, \"\"\n\t\t}\n\t\tif (line == p.Line \u0026\u0026 col \u003e p.Character) || line \u003e p.Line {\n\t\t\treturn 0, false, fmt.Sprintf(\"character %d is beyond line %d boundary\", p.Character, p.Line)\n\t\t}\n\t\toffset++\n\t\tif b == '\\n' {\n\t\t\tline++\n\t\t\tcol = 0\n\t\t} else {\n\t\t\tcol++\n\t\t}\n\t}\n\tif line == 0 {\n\t\treturn 0, false, fmt.Sprintf(\"character %d is beyond first line boundary\", p.Character)\n\t}\n\treturn 0, false, fmt.Sprintf(\"file only has %d lines\", line+1)\n}\n\nfunc rangeForNode(fset *token.FileSet, node ast.Node) lsp.Range {\n\tstart := fset.Position(node.Pos())\n\tend := fset.Position(node.End()) // node.End is exclusive, but we want inclusive\n\treturn lsp.Range{\n\t\tStart: lsp.Position{Line: start.Line - 1, Character: start.Column - 1},\n\t\tEnd:   lsp.Position{Line: end.Line - 1, Character: end.Column - 1},\n\t}\n}\n\ntype fakeNode struct{ p, e token.Pos }\n\nfunc (n fakeNode) Pos() token.Pos { return n.p }\nfunc (n fakeNode) End() token.Pos { return n.e }\n\nfunc goRangesToLSPLocations(fset *token.FileSet, nodes []*ast.Ident) []lsp.Location {\n\tlocs := make([]lsp.Location, len(nodes))\n\tfor i, node := range nodes {\n\t\tlocs[i] = goRangeToLSPLocation(fset, node.Pos(), node.End())\n\t}\n\treturn locs\n}\n\nfunc goRangeToLSPLocation(fset *token.FileSet, pos token.Pos, end token.Pos) lsp.Location {\n\treturn lsp.Location{\n\t\tURI:   \"file://\" + fset.Position(pos).Filename,\n\t\tRange: rangeForNode(fset, fakeNode{p: pos, e: end}),\n\t}\n\n}"}}
<-- result #2: textDocument/didOpen: {}
--> request #3: textDocument/documentSymbol: {"textDocument":{"uri":"file:///C:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/ast.go"}}
<-- result #3: textDocument/documentSymbol: []
--> request #4: textDocument/definition: {"textDocument":{"uri":"file:///C:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver/ast.go"},"position":{"character":30,"line":20}}
<-- error #4: textDocument/definition: {"code":0,"message":"import \"/C:/Users/prshrest/go/src/github.com/sourcegraph/go-langserver/langserver\": cannot import absolute path","data":null}

I get empty for textDocument/documentSymbol and error when using textDocument/definition.

@keegancsmith
Copy link
Member

Ok great, thanks for the logs. I should be able to follow up with that. I also have some experimental support to use AppVeyor to get some Windows CI :) Expect windows to be a bit more first class soon.

@keegancsmith
Copy link
Member

@prabirshrestha we will have proper windows support soon, @alexsaveliev has a working branch at #86 which you may want to try out.

@keegancsmith
Copy link
Member

The original rootPath issue has been fixed. Filed #113 to track progress on supporting windows.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants