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

Fix white spaces in project path #64

Merged
merged 2 commits into from
May 7, 2024
Merged

Fix white spaces in project path #64

merged 2 commits into from
May 7, 2024

Conversation

Sheol27
Copy link
Contributor

@Sheol27 Sheol27 commented Apr 12, 2024

Sorry, I had to delete the pull requests I made yesterday because of a problem! I hope this version using percent-encoding is fine, let me know!

@pr2502
Copy link
Owner

pr2502 commented Apr 14, 2024

hi. sorry for late response! i had a busy weekend. i've looked into it more and realized that this is unfortunately not enough to fix the percent encoding troubles because of false assumptions i made in the select_workspace_root function here and here.

i'm using paths directly as URIs without percent-encoding them which might break in case someone has (accidentally or on purpose) a file name with a % code in it.

specifically we'll need to move the URI destructuring into the select_workspace_root, percent-decode it there only where it's appropriate and return a path instead of URI (probably as a String). avoiding the roundtrips through the uriparse::URI type in branches where the input is not typed as URI coming from a LSP.

if you want to give it a go feel free to ask any more questions, or just leave it to me. i should be able to get to it within a few days^^

@pr2502
Copy link
Owner

pr2502 commented May 7, 2024

hi! sorry for the lack of progress. life got in the way.. I've implemented the change to only decode strings which are actually URIs and it should be working as expected now :)

@pr2502 pr2502 merged commit 74cf745 into pr2502:main May 7, 2024
1 check passed
@pr2502 pr2502 mentioned this pull request May 16, 2024
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 this pull request may close these issues.

2 participants