Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LSC does not encode/decode characters which are not allowed in File URIs #43

Closed
neowit opened this issue Nov 10, 2017 · 5 comments
Closed

Comments

@neowit
Copy link

neowit commented Nov 10, 2017

Hello @natebosch ,

Thank you for your plugin!

It would appear that current version of LSC does not encode characters which are not allowed in File URIs.

According to FILE URI requirements:

Characters which are not allowed in URIs, but which are allowed in filenames, must also be percent-encoded. For example, any of "{}`^ " and all control characters. In the example above, the space in the filename is encoded as %20.

For instance, here is a Go To Definition message from VS Code (which does appropriate encoding):

{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///Users/tester/projects/Sforce%20-%20SFDC%20Experiments/SForce%20%28lcs-test%29/src/classes/CompletionTester.cls"},"position":{"line":14,"character":10}}}

and here is the same message from vim LSC:

{"method":"textDocument/definition","jsonrpc":"2.0","id":2,"params":{"textDocument":{"uri":"file:///Users/tester/projects/Sforce - SFDC Experiments/SForce (lcs-test)/src/classes/CompletionTester.cls"},"position":{"character":10,"line":14}}}

In the opposite direction - if server does appropriate URI encoding and returns URI same way as in above VS Code example - vim LSC does not understand returned path.

e.g. example below is understood by VS Code but not understood by vim LSC

{"id":1,"result":[{"uri":"file:///Users/tester/projects/Sforce%20-%20SFDC%20Experiments/SForce%20(lsc-test)/src/classes/CompletionTester.cls","range":{"start":{"line":10,"character":8},"end":{"line":10,"character":18},"offset":{"line":0,"character":0}}}],"jsonrpc":"2.0"}

@natebosch
Copy link
Owner

Curiously in your last example the () are not encoded...

I think I can go with a relatively straightforward encode strategy since we expect the entire file path to be treated as the path component from the URI spec.

  1. Allow unreserved characters
  2. Allow / unescaped
  3. Escape all other characters. Expect that all characters are ascii

My first approach for decoding will probably only handle %nn style encodings. Handling other cases like translating + to space or %% to % could be trickier since it's less general.

@natebosch
Copy link
Owner

Can you try updating to the latest and see if it works for you?

@neowit
Copy link
Author

neowit commented Nov 11, 2017

Hello @natebosch ,

Thank you for the update, looks good.
URI encoding for messages like textDocument/definition or textDocument/didOpen appears to be working, but there is one case (I found so far) where URI encoding does not get applied:

{"method":"initialize","jsonrpc":"2.0","id":1,"params":{"rootUri":"file:///Users/tester/projects/Sforce - SFDC Experiments/SForce (lsc-test)/src/classes","capabilities":{...}}

I think this line:

      \ 'rootUri': 'file://'.getcwd(),

should look something like like this:

      \ 'rootUri': lsc#uri#documentUri(getcwd()),

Curiously in your last example the () are not encoded...

that's my bad. Thank you for highlighting this.

natebosch added a commit that referenced this issue Nov 11, 2017
Fixes remaining issue in #43
@natebosch
Copy link
Owner

Good catch on the rootUri. Should be fixed now.

@neowit
Copy link
Author

neowit commented Nov 12, 2017

That's great, thank you.

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

No branches or pull requests

2 participants