Fix #1020 #1021
Merged
Fix #1020 #1021
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
frarees
force-pushed
the
uri_encode
branch
2 times, most recently
from
a59a183 to
66c2fcd
Compare
|
@frarees why did you remove the decoding check? This will make URI with spaces in them not work again I think. You found the bug ( As for the regression test, let's add that later and fix this issue for now. |
|
For some reason, I thought not encoding (the original fix) broke compatibility (#1020). Added it back. |
|
Yes I think what broke was just the bug of assigning |
ychin
added a commit
to ychin/macvim
that referenced
this pull request
Jul 6, 2020
Change mvim:// protocol behavior back to previous state to double-encode the file path, since we are encapsulating a file:// protocol (which has encoded path) within another URL as a query, which itself should be encoded. This means a file path "/tmp/file name.txt" should be properly encoded as mvim://open?url=file:///tmp/file%2520name.txt, as the space is encoded twice (first to %20, then to %2520). Previously we tried to fix the protocol handler to only do a single encoding (see macvim-dev#1021 and macvim-dev#1043) but it's really an incorrect usage of URL. The reason for that fix was that tools like iTerm2 was passing in single-encoded URLs. As such, also add a compatibility feature here where we will optimiscally try to re-encode characters that we detect to be erroneously encoded. For example, if we see mvim://open?url=file:///tmp/file%20name.txt, MacVim will intelligently realize that the space needs to be encoded again. The only character where that won't work is the "%" character because of the ambiguity involved, so a file path "/tmp/file%.txt" will only work with this: mvim://open?url=file:///tmp/file%2525.txt Close macvim-dev#1020 (also see the issue for discussions).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #1002 removed decoding of the mvim:// url query.
Every query goes through decoding now, no exceptions.
This should make it work the way it was.
We can use this PR to generate regression tests as well, as pointed here #1020 (comment)