WIP (Don't merge): Fix external editor URL scheme improperly encoding file URLs

[ychin]

Fixes how iTerm2 handles encoding for the file protocol URL it uses to spawn external editors such as TextMate or MacVim. In particular, previously the slashes in the file:// URL were all encoded to "%2F", which would be incorrect as it resulted in paths such as file://%2FUsers%2Ffoo%2Ffile%20with%20space. The URL is invalid in that case and it's an accident that it worked with existing editors.

Fix this by using the proper character set to encode (path) instead of the original "host" configuration.

Also see macvim-dev/macvim#1020.

[ychin]

Hi, just to elaborate on the PR more. I'm the maintainer of MacVim and a short while ago we fixed our mvim:// protocol handling to work better with space and other special chars, but it broke iTerm2 integration. I and working around it but I think this has to do with iTerm2's way of escaping the URL.

Basically to recap, the API is supposed to be mvim://open?url=file:///foo/bar/file%20with%20space.txt. The way iTerm2 escapes it results in it looking like mvim://open?url=file://%2Ffoo%2Fbar%2Ffile%20with%20space.txt, which is not valid, as file://%2Ffoo%2Fbar%2Ffile%20with%20space.txt is not a proper file protocol URL. If you decode the URL, you get file:///foo/bar/file with space.txt which is also not valid because now the space is not encoded to %20. I think we should just encode it with the correct parameter so that we get well-formed URLs. Given how most text editors kind of worked before I'm suspecting they were just manually parsing the URL instead of using NSURL, which is what my custom workaround does, but I think it would be nice if we try to form well-formed URLs here to make it more cross-compatible.

[ychin]

Changed title to WIP because there are some discussions on macvim-dev/macvim#1020 where we are trying to decide on the encoding scheme.

[gnachman]

I haven't read the whole thread on macvim, but on its face this seems reasonable. The only reason that host escaping was chosen instead of path was to get backward compatibility with the execrable stringByAddingPercentEscapesUsingEncoding: of old. My only concern is that it will may break support for one of the other editors that take that code path, but I suspect it will be a net positive change.

[gnachman]

…looks like only BBEdit, TextMate, and MacVim take that code path so it won't be too bad to test.

[miks]

@gnachman are there any hopes that this will be merged? I'm currently using 161 due to this problem with iTerm.

[ychin]

Sorry after some discussion I actually have to revert the change on MacVim and update this PR to use the new way. See the linked issue (at #419 (comment)) for discussion. I have been busy with work and other things but need to get to it.

But yeah that's the reason why I didn't change the title from "WIP (Don't merge)".

[gnachman]

@ychin This PR seems obviously correct to me. Is there any reason merging it could cause harm for users of either old or new or forseeable future versions of macVim?