Fix mvim:// protocol handler not working well with iTerm2 #1043
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
iTerm2's MacVim integration generates URLs that has the file separator
("/") escaped as ("%2F"). This is incorrect (as it implies the slash is
part of the file name itself instead of a separator), but work around it
for now. Instead of using NSURL to parse the file:// URL, just decode
the input parameter and manually parse it. This still handles the
special characters like space or "?", but will handle the escaped
slashes as well. Given that "/" is not a valid filename character we
should not run into ambiguity here.
Also, add dialog boxes to show an error if MacVim doesn't know how to
handle a file path.
Will add regression tests for these edge cases in a future commit.
Fix macvim-dev#1020
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.
iTerm2's MacVim integration generates URLs that has the file separator ("/") escaped as ("%2F"). This is incorrect (as it implies the slash is part of the file name itself instead of a separator), but work around it for now. Instead of using NSURL to parse the file:// URL, just decode the input parameter and manually parse it. This still handles the special characters like space or "?", but will handle the escaped slashes as well. Given that "/" is not a valid filename character we should not run into ambiguity here.
Also, add dialog boxes to show an error if MacVim doesn't know how to handle a file path.
Will add regression tests for these edge cases in a future commit.
Fix #1020