Open tapayne88 opened 3 years ago
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?
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
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.
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!
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.
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?
Ah, that's awesome!
One thing I'm having difficultly with is that URI encoding uses
%
which vim seems to expand with inexpand("<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
.
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
This repo might be of use with that above https://github.com/tapayne88/yarn-pnp-demo
I think these 2 lines are causing this issue. Do you think we could replace
fn
witha: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.
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 🤔
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.
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
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:
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?
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.
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.
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.
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.
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()
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.
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.
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.
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
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.
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 @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 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.
@tapayne88 any idea what the eta is on 0.6?
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
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()
- [neovim#16340/runtime/autoload/zip.vim](https://github.com/neovim/neovim/pull/16340/files#diff-c3431e35dce9ef0098df28d227c4539fc744a55e32dca0291be3eeddab0fe245R212-R216)
- original vim patch - https://github.com/vim/vim/commit/519cc559b08b800edc429688aece7ad6a00d41eb
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 `
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.
Hey, @tapayne88 , I finally got into LSP and node (switched jobs), and want to integrate URL decoding into vim-rzip. I tried replicating the issue with an example yarn project, but I can't get it to compress cached packages.
I tried creating a .yarnrc.yml with compressionLevel: 9
or compressionLevel: "9"
, but it was no good.
Hello, what's the status on this? I am still unable to open zipfile paths in my neovim setup.
@guigui64 can you guide on how to get a yarn project to compress its cache into zip files? And please show me to which definition you're trying to jump and what error you're getting.
Hi 👋
I'm wondering what your view is on supporting the URI decoding of filenames. I'm trying open filenames like this
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:
What do you think?