vim-rzip icon indicating copy to clipboard operation
vim-rzip copied to clipboard

Support for URI decoding

Open tapayne88 opened this issue 4 years ago • 33 comments

Hi 👋

I'm wondering what your view is on supporting the URI decoding of filenames. I'm trying open filenames like this

/Users/thomas.payne/git/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts

I have a little function that I can wrap these up with but it would be super handy if vim-rzip could do the URI decoding for me that way I could drop my custom URI scheme yarnpnp:

function! YarnPnPRead(uri)
    let l:parsed_uri = substitute(a:uri, '%\([a-fA-F0-9][a-fA-F0-9]\)', '\=nr2char("0x" . submatch(1))', "g")
    let l:zipfile_uri = substitute(l:parsed_uri, '^yarnpnp', 'zipfile', "g")
    call rzip#Read(l:zipfile_uri, 1)
endfunction

augroup YarnPnP
    autocmd!
    autocmd BufReadCmd yarnpnp:*,yarnpnp:*/* call YarnPnPRead(expand('<amatch>'))
augroup END

What do you think?

tapayne88 avatar Jul 02 '21 14:07 tapayne88

I had to research the terms, because I'm not familiar with OSX. By custom URI scheme you I assume you mean OSX's handling of custom protocols.

Unfortunately, Vim does not handle URI decoding, so I don't think this plugin should take such a task. There's tpope's vim-unimpaired, that has mappings for encoding and decoding URLs. Maybe that would suit you better?

lbrayner avatar Jul 03 '21 00:07 lbrayner

Thanks for your response!

Sorry, I should have given more context. I'm trying to get the typescript language server to support yarn PnP projects in neovim using native LSP. I have a few PRs open in a few places to try to do it

  • https://github.com/theia-ide/typescript-language-server/pull/220
  • https://github.com/yarnpkg/berry/pull/3041
  • https://github.com/neovim/neovim/pull/14959

The tl;dr is typescript (with a bit of help from yarn) can emit file identifiers like this

zipfile:/Users/thomas.payne/git/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts

which use a URI scheme zipfile. Due to the URI encoding (%40 & %3A) this file can't be opened by vim-rzip but it can if decoded.

My workaround at the minute is to have yarn set the file identifiers to yarnpnp scheme so I can URI decode them before calling your plugin

yarnpnp:/Users/thomas.payne/git/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts

becomes

zipfile:/Users/thomas.payne/git/yarn-pnp-demo/.yarn/cache/@testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip::node_modules/@testing-library/react/types/index.d.ts

Your plugin already does 99% of what I need and thought it isn't too much of a stretch to include URI decoding but I very much defer to your opinion on that 😄.

I guess I could use vim-unimpaired's decode algorithm but I would still need to have a custom function / autocmd to handle the file loading.

tapayne88 avatar Jul 05 '21 08:07 tapayne88

You can hack vim-rzip with the help of the VimEnter event. I think I was able to to solve your problem with this:

function! ParseURI(uri)
    return substitute(a:uri, '%\([a-fA-F0-9][a-fA-F0-9]\)', '\=nr2char("0x" . submatch(1))', "g")
endfunction

function! RzipOverride()
    autocmd! zip BufReadCmd   zipfile:*/*
    exe "au! zip BufReadCmd ".g:zipPlugin_ext
    autocmd zip BufReadCmd   zipfile:*/* call rzip#Read(ParseURI(expand("<amatch>")), 1)
    exe "au zip BufReadCmd ".g:zipPlugin_ext." call rzip#Browse(ParseURI(expand('<amatch>')))"
endfunction

autocmd VimEnter * call RzipOverride()

I tested it with a file named nested.zip inside @zipinzip.zip, with the ex command e zipfile:/home/desenvolvedor/Downloads/\%40zipinzip.zip::nested.zip, the contents of which I was able to edit.

See if that works for you!

lbrayner avatar Jul 06 '21 01:07 lbrayner

As I was testing, I forgot to replace the colons with 3A (yielding e zipfile:/home/desenvolvedor/Downloads/\%40zipinzip.zip%3A%3Anested.zip). When I did, the write command failed! I'll look into that soon.

lbrayner avatar Jul 06 '21 01:07 lbrayner

Ah, that's awesome!

One thing I'm having difficultly with is that URI encoding uses % which vim seems to expand with in expand("<amatch>") calls to the file path. Do you know a way to prevent this?

tapayne88 avatar Jul 06 '21 15:07 tapayne88

Ah, that's awesome!

One thing I'm having difficultly with is that URI encoding uses % which vim seems to expand with in expand("<amatch>") calls to the file path. Do you know a way to prevent this?

Could you give me an example of you're trying to do? For example, if it's an ex command, such as e zipfile:/home/desenvolvedor/Downloads/\%40zipinzip.zip::nested.zip, note that the percent sign is escaped. If it's an evaluated expression, such as exe "e " . fnameescape('zipfile:/home/desenvolvedor/Downloads/%40hi.zip%3A%3Anested.zip'), you can use fnameescape.

lbrayner avatar Jul 06 '21 21:07 lbrayner

So I think I might have been misunderstanding what was happening.

I've been using this command to debug

:exe "e ".fnameescape("zipfile:/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts")

When listing buffers (:ls) I see something like

  2 %a   "zipfile:/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/zipfile:/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-l
ibrary/react/types/index.d.ts40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zipzipfile:/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%4
0testing-library/react/types/index.d.ts3Azipfile:/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts3Anode_mo
dules/zipfile:/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts40testing-library/react/types/index.d.ts" line 1

I think these 2 lines are causing this issue. Do you think we could replace fn with a:fname? https://github.com/lbrayner/vim-rzip/blob/b67c1a5aa4467d5784f03599ed3eb756ae07a3c6/autoload/rzip.vim#L67 https://github.com/lbrayner/vim-rzip/blob/b67c1a5aa4467d5784f03599ed3eb756ae07a3c6/autoload/rzip.vim#L73

tapayne88 avatar Jul 07 '21 14:07 tapayne88

This repo might be of use with that above https://github.com/tapayne88/yarn-pnp-demo

tapayne88 avatar Jul 07 '21 15:07 tapayne88

I think these 2 lines are causing this issue. Do you think we could replace fn with a:fname?

These lines are in upstream, but don't worry, I found a solution. The trick is the order in which autocommands are added. One needed only to execute exe "keepalt file " . fnameescape(ParseURI(expand("<amatch>"))) before doing call rzip#Read(expand("<amatch>"), 1), which is in the original autocommand, like so:

function! ParseURI(uri)
    return substitute(a:uri, '%\([a-fA-F0-9][a-fA-F0-9]\)', '\=nr2char("0x" . submatch(1))', "g")
endfunction

function! RzipOverride()
    autocmd! zip BufReadCmd   zipfile:*/*
    exe "au! zip BufReadCmd ".g:zipPlugin_ext
    autocmd zip BufReadCmd   zipfile:*/* exe "keepalt file " . fnameescape(ParseURI(expand("<amatch>")))
    autocmd zip BufReadCmd   zipfile:*/* call rzip#Read(expand("<amatch>"), 1)
    exe "au zip BufReadCmd ".g:zipPlugin_ext." call rzip#Browse(ParseURI(expand('<amatch>')))"
endfunction

autocmd VimEnter * call RzipOverride()

I just quickly tested this with much success. See if works for you.

lbrayner avatar Jul 07 '21 22:07 lbrayner

It almost works 😄 Found the rzip#Read call still needs to use ParseURI but otherwise it works!

function! ParseURI(uri)
    return substitute(a:uri, '%\([a-fA-F0-9][a-fA-F0-9]\)', '\=nr2char("0x" . submatch(1))', "g")
endfunction

function! RzipOverride()
    autocmd! zip BufReadCmd zipfile:*,zipfile:*/*
    exe "au! zip BufReadCmd ".g:zipPlugin_ext

    autocmd zip BufReadCmd zipfile:*,zipfile:*/* exe "keepalt file " . fnameescape(ParseURI(expand("<amatch>")))
    autocmd zip BufReadCmd zipfile:*,zipfile:*/* call rzip#Read(ParseURI(expand('<amatch>')), 1)
    exe "au zip BufReadCmd ".g:zipPlugin_ext." call rzip#Browse(ParseURI(expand('<amatch>')))"
endfunction

autocmd VimEnter * call RzipOverride()

I think the last thing I'm struggling (and I'm not sure it's vim-rzip's problem) is other bits of code trying to load the same file multiple times, it causes an error Buffer with this name already exists. Neovim's native LSP does it when trying to open a definition location (here). This function has already opened the file in a buffer (under the parsed named) however when it tries to load the filename again (prior to URI deocding) our autocommand kicks in and tries rename to the already open buffer - hence the error.

I'm not sure how to handle instance like this as we only rename the buffer when it's read 🤔

tapayne88 avatar Jul 08 '21 16:07 tapayne88

other bits of code trying to load the same file multiple times, it causes an error Buffer with this name already exists

Yes, this is a shortcoming of the bundled in zip.vim, which I did not address in vim-rzip. Both vanilla zip.vim and gzip.vim's supported use case was just quickly browsing an archive (and possibly even editing a text file in it) with Vim by running vim archive.zip from the command and then closing it. If you run a long session you will certainly encounter bugs.

If I think of something I'll let you know, since this would be very useful to me, because I run long and persistent sessions with the help of vim-obsession. I haven't started using LSPs yet, but hope to soon.

lbrayner avatar Jul 09 '21 01:07 lbrayner

Cool, thanks so much for your help! I'm building up my vim understanding and I really appreciate you taking the time to answer my questions and help debug!

Do you think the above autocommands make sense to be incorporated into vim-rzip behind a feature switch or perhaps in the readme?

let g:rzip_enable_uri_parsing = v:true

tapayne88 avatar Jul 09 '21 15:07 tapayne88

Do you think the above autocommands make sense to be incorporated into vim-rzip behind a feature switch or perhaps in the readme?

I'd have to think a little more about it.

other bits of code trying to load the same file multiple times, it causes an error Buffer with this name already exists.

Part of the solution would be changing this line:

https://github.com/lbrayner/vim-rzip/blob/b67c1a5aa4467d5784f03599ed3eb756ae07a3c6/autoload/rzip.vim#L288

into

setlocal bufhidden=wipe

This will wipe the buffer as soon as it's hidden, thus avoiding Buffer with this name already exists.

But there's the case when the buffer is still being displayed, i.e. not hidden — perhaps in another tab. It has to be wiped or focused, which implies switching windows or possibly even tabs. What do you think? Do you think that's too heavy handed?

lbrayner avatar Jul 09 '21 16:07 lbrayner

I'm not sure that will help because I think the error is thrown when we attempt to rename the buffer to the parsed name. Neovim internals are all handling filenames like this

/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts

They look for already open buffers with that name - in some scenarios there may be one but it'll likely be empty (or unloaded. Neovim will try to create it and when our autocommand runs it'll rename it causing the error.

I think it might help in the scenario where neovim finds a matching buffer (incorrect one) and loads it, this will show an empty buffer.

I was wondering if we could in the BufReadCmd autocommand detect any open buffers with our target name and return that rather than opening a new one.

tapayne88 avatar Jul 12 '21 11:07 tapayne88

Since I'm not currently using Neovim's LSP features, I'm a bit confused as to the complete picture of your use case.

I'm not sure that will help because I think the error is thrown when we attempt to rename the buffer to the parsed name. Neovim internals are all handling filenames like this

/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts

So Neovim's internals are handling file names like that. Do you mean a URI such as file:///Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts? Because it doesn't make sense to me if it's only /Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts without some protocol (like file://).

From this answer:

The tl;dr is typescript (with a bit of help from yarn) can emit file identifiers like this

zipfile:/Users/thomas.payne/git/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts

From that point on, I thought your issue was limited to URI's with the zipfile: protocol.

lbrayner avatar Jul 13 '21 22:07 lbrayner

Apologies! Yeah so neovim's LSP gets URIs like these from the server

zipfile:/Users/thomas.payne/git/yarn-pnp-demo/.yarn/cache/%40testing-library-react-npm-11.2.7-3a0469c756-389c9f3e83.zip%3A%3Anode_modules/%40testing-library/react/types/index.d.ts

The bit I'm trying to diagnose is jump-to-definition, so from a symbol / keyword neovim asks the LSP for it's definition location. The LSP can return 0, 1 or more locations. If it has a single location neovim attempts to load the instance into a new buffer and place the cursor in the correct position. This works as expected - no errors. If there are multiple then neovim opens the first instance and tries to grab the relevant line for each match to show in a jump list. It seems to be here where the issue occurs because the first buffer is already open when it attempts to grab the lines for the jump list.

tapayne88 avatar Jul 14 '21 20:07 tapayne88

The bit I'm trying to diagnose is jump-to-definition, so from a symbol / keyword neovim asks the LSP for it's definition location. The LSP can return 0, 1 or more locations. If it has a single location neovim attempts to load the instance into a new buffer and place the cursor in the correct position. This works as expected - no errors. If there are multiple then neovim opens the first instance and tries to grab the relevant line for each match to show in a jump list. It seems to be here where the issue occurs because the first buffer is already open when it attempts to grab the lines for the jump list.

This is the same problem you described previously, and you pointed out the exact line of code that's giving you the error:

Neovim's native LSP does it when trying to open a definition location (here). This function has already opened the file in a buffer (under the parsed named) however when it tries to load the filename again (prior to URI deocding) our autocommand kicks in and tries rename to the already open buffer - hence the error.

On line 1519 there's a call to bufload, after which, as you said, our autocommand kicks in and tries rename to the already open buffer. This is exactly what I was trying to address by wiping the buffer in my answer.

This can be accomplished thus:

function! ParseURI(uri)
    return substitute(a:uri, '%\([a-fA-F0-9][a-fA-F0-9]\)', '\=nr2char("0x" . submatch(1))', "g")
endfunction

function! RzipOverride()
    autocmd! zip BufReadCmd   zipfile:*/*
    exe "au! zip BufReadCmd ".g:zipPlugin_ext
    autocmd zip BufReadCmd   zipfile:*/*
                \ if ParseURI(expand("<amatch>")) !=# expand("<amatch>") |
                \     if bufexists(ParseURI(expand("<amatch>"))) |
                \         sil! exe "bwipeout " . fnameescape(ParseURI(expand("<amatch>"))) |
                \     endif |
                \     exe "keepalt file " . fnameescape(ParseURI(expand("<amatch>"))) |
                \     if bufexists(expand("<amatch>")) |
                \         exe "bwipeout " . fnameescape(expand("<amatch>")) |
                \     endif |
                \ endif
    autocmd zip BufReadCmd   zipfile:*/* call rzip#Read(expand("<amatch>"), 1)
    exe "au zip BufReadCmd ".g:zipPlugin_ext." call rzip#Browse(ParseURI(expand('<amatch>')))"
endfunction

autocmd VimEnter * call RzipOverride()

When you execute :file, an unlisted buffer is created to hold the old name, and that's what was causing the error Buffer with this name already exists. The code above will wipe those unlisted buffers.

It's not very elegant, but didactic, because I was avoiding state and creating other functions.

lbrayner avatar Jul 15 '21 01:07 lbrayner

This would be a more compact version, but a few potential errors would be silenced and ignored:

function! ParseURI(uri)
    return substitute(a:uri, '%\([a-fA-F0-9][a-fA-F0-9]\)', '\=nr2char("0x" . submatch(1))', "g")
endfunction

function! RzipOverride()
    autocmd! zip BufReadCmd   zipfile:*/*
    exe "au! zip BufReadCmd ".g:zipPlugin_ext
    autocmd zip BufReadCmd   zipfile:*/*
                \ if ParseURI(expand("<amatch>")) !=# expand("<amatch>") |
                \     sil! exe "bwipeout " . fnameescape(ParseURI(expand("<amatch>"))) |
                \     exe "keepalt file " . fnameescape(ParseURI(expand("<amatch>"))) |
                \     sil! exe "bwipeout " . fnameescape(expand("<amatch>")) |
                \ endif
    autocmd zip BufReadCmd   zipfile:*/* call rzip#Read(expand("<amatch>"), 1)
    exe "au zip BufReadCmd ".g:zipPlugin_ext." call rzip#Browse(ParseURI(expand('<amatch>')))"
endfunction

autocmd VimEnter * call RzipOverride()

lbrayner avatar Jul 15 '21 16:07 lbrayner

Just pushed https://github.com/lbrayner/vim-rzip/commit/5e7015040c177d07a95e2f15b691f129f158376a, which sets 'bufhidden' to wipe (as I discussed previously) and fixes a bug exposed by multiple attempts to edit the same zipfile: URI.

lbrayner avatar Jul 15 '21 16:07 lbrayner

Just pushed 5e70150, which sets 'bufhidden' to wipe (as I discussed previously) and fixes a bug exposed by multiple attempts to edit the same zipfile: URI.

Awesome, thank you!

This does look to work for my needs as I use telescope to handle the definition lookup. Annoyingly the default handling in neovim opens the first found definition location (not so with telescope) so when the jump list is created we're trying to bwipeout the current in-focus buffer 😬 - otherwise works great!

One thing I do find is that this line

autocmd zip BufReadCmd   zipfile:*/* call rzip#Read(expand("<amatch>"), 1)

still needs to call ParseURI

autocmd zip BufReadCmd   zipfile:*/* call rzip#Read(ParseURI (expand("<amatch>")), 1)

Last thing I'm scratching my head over is is how to flag the new buffer with # so I can quickly swap back and forth.

tapayne88 avatar Jul 15 '21 19:07 tapayne88

Last thing I'm scratching my head over is is how to flag the new buffer with # so I can quickly swap back and forth.

Maybe you could show me what you're doing with asciinema. I recorded a showcase of the new code: https://asciinema.org/a/cXtfR4xMyyXhPnD0eQgfLWSTY.

lbrayner avatar Jul 15 '21 19:07 lbrayner

Here's the flow I'm using. You can see :b # working with a file within the repo itself but it doesn't work for files opened via vim-rzip. https://asciinema.org/a/425697

tapayne88 avatar Jul 15 '21 21:07 tapayne88

Hey, I'm curious as to how the LSP code was able to display the recursively zipped *.ts files (when you run Telescope lsp_definitions on import { render...}) with just the URI. Was it the code or did you open them manually beforehand? Because vim-rzip is not capable of loading a recursively zipped file directly with the URI, you have to navigate manually.

While I was investigating the alternate buffer issue (:b#) I found a bug in the netrw integration of the upstream code, which I'll try to tackle.

lbrayner avatar Jul 20 '21 12:07 lbrayner

As I understand it telescope uses neovim's LSP util locations_to_items (calling it here) to load the buffer in the background and grab a preview line from it (ultimately here). This is what causes the duplicate buffer error in the default neovim LSP handling for definition as the buffer has already been opened when the preview line are grabbed.

tapayne88 avatar Jul 23 '21 14:07 tapayne88

@tapayne88 @lbrayner Hey so I've got everything set up as described on https://yarnpkg.com/getting-started/editor-sdks#vim but when I try to jump to definition with a package thats not part of the current project, I get

Error executing vim.schedule lua callback: ...l/Cellar/neovim/0.5.0/share/nvim/runtime/lua/vim/uri.lua:116: URI must contain a scheme: zipfile:/Users/benwainwright1/repos/tnm/.yarn/cache/%40types-deep-equal-npm-1.0.1-c01c817a94-689b5737dd.zip%3A%3Anode_modules/%40types/deep-equal/index.d.ts 

benwainwright avatar Aug 14 '21 16:08 benwainwright

@benwainwright ah! So unfortunately that is a bug in neovim's URI handling that is fixed but missed the v0.5 release which means it's only available in the nightly builds. I'm afraid you'll need to use nightly to have things like jump-to-definition work or wait for v0.6.

https://github.com/neovim/neovim/pull/14959

tapayne88 avatar Aug 16 '21 09:08 tapayne88

@tapayne88 any idea what the eta is on 0.6?

benwainwright avatar Aug 16 '21 09:08 benwainwright

Afraid I don't know @benwainwright. Sounds like v0.5.1 will come sooner but I think the required change needs backporting to be included (I'm not familiar with the process) otherwise it'll have to wait for v0.6

tapayne88 avatar Aug 16 '21 12:08 tapayne88

So, Neovim v0.6 is here and there is some fun stuff in there, some from vim upstream. I think a change to vim's builtin zip handling is the culprit.

tl;dr change instances of <amatch> to <afile> in autocommands and everything should work, for now

" Decode URI encoded characters
function! DecodeURI(uri)
    return substitute(a:uri, '%\([a-fA-F0-9][a-fA-F0-9]\)', '\=nr2char("0x" . submatch(1))', "g")
endfunction

" Attempt to clear non-focused buffers with matching name
function! ClearDuplicateBuffers(uri)
    " if our filename has URI encoded characters
    if DecodeURI(a:uri) !=# a:uri
        " wipeout buffer with URI decoded name - can print error if buffer in focus
        sil! exe "bwipeout " . fnameescape(DecodeURI(a:uri))
        " change the name of the current buffer to the URI decoded name
        exe "keepalt file " . fnameescape(DecodeURI(a:uri))
        " ensure we don't have any open buffer matching non-URI decoded name
        sil! exe "bwipeout " . fnameescape(a:uri)
    endif
endfunction

function! RzipOverride()
    " Disable vim-rzip's autocommands
    autocmd! zip BufReadCmd   zipfile:*,zipfile:*/*
    exe "au! zip BufReadCmd ".g:zipPlugin_ext

    " order is important here, setup name of new buffer correctly then fallback to vim-rzip's handling
    autocmd zip BufReadCmd   zipfile:*  call ClearDuplicateBuffers(expand("<afile>"))
    autocmd zip BufReadCmd   zipfile:*  call rzip#Read(DecodeURI(expand("<afile>")), 1)

    if has("unix")
        autocmd zip BufReadCmd   zipfile:*/*  call ClearDuplicateBuffers(expand("<afile>"))
        autocmd zip BufReadCmd   zipfile:*/*  call rzip#Read(DecodeURI(expand("<afile>")), 1)
    endif

    exe "au zip BufReadCmd ".g:zipPlugin_ext."  call rzip#Browse(DecodeURI(expand('<afile>')))"
endfunction

autocmd VimEnter * call RzipOverride()
Detail

From what I can tell vim/neovim now expect zipfiles to use the zipfile:// scheme (not zipfile:)

  • :e zipfile:/Users/... doesn't work
  • :e zipfile:///Users/... does work

This looks to have had a knock on affect on how <amatch> gets expanded. The documentation change kind of hints at what's happening

When the match is with a file name, it is expanded to the full path.

https://github.com/vim/vim/commit/4700398e384f38f752b432e187462f404b96847d#diff-4188ca60fe651289f55943772fd6b6b867e721976d75f8be575ecb98ad30e4cbR922-R926

I've found this to mean files opened as zipfile:/Users/... won't be open. In vim's debug.log (using vim -V9debug.log) you see the <amatch> filename become relativised against the directory you're in

chdir(/Users/thomas.payne/git/github.com/yarn-pnp-demo/zipfile:/Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/@testing-library-dom-npm-7.31.2-d66ba6a14d-f930b4797f.zip::node_modules/@testing-library/dom/types)
...
:!unzip -p -- '/Users/thomas.payne/git/github.com/yarn-pnp-demo//Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/@testing-library-dom-npm-7.31.2-d66ba6a14d-f930b4797f.zip' '/Users/thomas.payne/git/github.com/yarn-pnp-demo/node_modules/@testing-library/dom/types/screen.d.ts' > /var/folders/jy/zvr7nwjx3gsfg46ftg0qn3kc0000gp/T/nvimd2hcGv/1

Despite handling seeming to continue to work when using zipfile:///Users/... I do wonder if this might cause issues for windows users as with things as they are the underlying zip command becomes

:!unzip -p -- '///Users/thomas.payne/git/github.com/yarn-pnp-demo/.yarn/cache/@testing-library-dom-npm-7.31.2-d66ba6a14d-f930b4797f.zip' 'node_modules/@testing-library/dom/types/screen.d.ts' > /var/folders/jy/zvr7nwjx3gsfg46ftg0qn3kc0000gp/T/nvimhjdbL9/1

Perhaps incorporating a similar change to what vim's zip handling into vim-rzip is needed but this has implications on editor versions and also doesn't currently play nice with the way URIs work in language servers like typescript-language-server

Sorry this is so long! Appreciate it if you got here!

tapayne88 avatar Dec 08 '21 15:12 tapayne88

Hey, @tapayne88, I just pushed a commit which restructures the code, basing it on a fixed zip.vim version (v28) and fixing the URI scheme so that it works on Neovim 0.6.1.

Please let me know if you encounter any problems.

lbrayner avatar Feb 17 '22 20:02 lbrayner