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

In internal/lsp/text/edit.go, ToPath doesn't percent decode input URI #60

Open
alurm opened this issue Sep 15, 2023 · 2 comments · May be fixed by #61
Open

In internal/lsp/text/edit.go, ToPath doesn't percent decode input URI #60

alurm opened this issue Sep 15, 2023 · 2 comments · May be fixed by #61

Comments

@alurm
Copy link

alurm commented Sep 15, 2023

Here's an example.

On my machine, C++'s std::deque is located at this path:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/deque

I tried doing "L def" on std::deque.

Since the path contains ++ ("c++"), when it got to Acme, it was interpreted as this filename:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c%2B%2B/v1/deque

Notice percent encoded %2B%2B.

Since this file doesn't exist, this error is printed:

can't open /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c%2B%2B/v1/deque: No such file or directory

@alurm
Copy link
Author

alurm commented Sep 15, 2023

Here is a diff to quickly fix this:

diff --git a/internal/lsp/text/edit.go b/internal/lsp/text/edit.go
index 3848358..177f87b 100644
--- a/internal/lsp/text/edit.go
+++ b/internal/lsp/text/edit.go
@@ -4,6 +4,7 @@ package text
 import (
 	"fmt"
 	"io"
+	"net/url"
 	"sort"
 	"strings"
 
@@ -127,6 +128,7 @@ func ToURI(filename string) protocol.DocumentURI {
 // ToPath converts URI to filename.
 func ToPath(uri protocol.DocumentURI) string {
 	filename, _ := CutPrefix(string(uri), "file://")
+	filename, _ = url.PathUnescape(filename)
 	return filename
 }
 

@alurm
Copy link
Author

alurm commented Sep 15, 2023

It may be the case that something about ToURI should be done as well.

It currently doesn't decode escape the path.

I tried defining ToURI as such:

// ToURI converts filename to URI.
func ToURI(filename string) protocol.DocumentURI {
	return protocol.DocumentURI("file://" + url.PathEscape(filename))
}

This doesn't work. I think it's because url.PathEscape escapes "/" as well.

alurm added a commit to alurm/acme-lsp that referenced this issue Sep 15, 2023
This is useful in cases where initial path contains special characters.

For example, path containing "c++" needs to be properly decoded, otherwise Acme will try to open file containing %2B%2B, which is wrong.

Issue 9fans#60 on GitHub.
@alurm alurm linked a pull request Sep 15, 2023 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant