[vim/vim] zipfile opens an empty buffer after 8.2.3468 (#8974)

[依云]

To Reproduce
Detailed steps to reproduce the behavior:

1. Run vim xxx.zip
2. Select one entry inside the zip listing and press Enter to open/view it
3. An empty buffer is open. Buffer name is ~/.../zipfile:/.../xxx.zip:xxx.txt, i.e. with current directory prepended.

Expected behavior
netrw should read the zipped xxx.txt's content into new buffer.

Environment (please complete the following information):

• Vim version: from 8.2.3468 (found by git bisect, first bad commit: c6376c7) to 8.2.3486 (latest)
• OS: Arch Linux
• Terminal: GNOME Terminal

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.

[Shane-XB-Qian]

looks it's not related to 'netrw' but 'zip.vim', though same auther: @cecamp
// looks 'zip.vim' was not a complete runtime, it set ft of zip to 'tar' also?!......

[cecamp]

That is odd. It was working when I was working with an earlier version of vim, then it stopped working when I myself updated vim. I keep a copy of my plugins separate from vim, so they're not affected by updates. I see what Shane means; opening the zip file shows a filetype of tar, not zip. Putting a line into my own filetype.vim such as

au BufNewFile,BufReadPost *.zip	setf zip

still leaves *.zip files being recognized as *.tar files. I don't think this has anything to do with something that I did.

[Christian Brabandt]

Hm, the tar filetype come from this line in the plugin:

https://github.com/vim/vim/blob/master/runtime/autoload/zip.vim#L118

I don't think this is correct. For the other issue, let me have a quick look

[Charles Campbell]

Christian Brabandt (Vim Github Repository) wrote:
>
> Hm, the tar filetype come from this line in the plugin:
>
> https://github.com/vim/vim/blob/master/runtime/autoload/zip.vim#L118
>
> I don't think this is correct. For the other issue, let me have a
> quick look
>
>

Thanks! I'd completely forgotten about it (set ft=tar) and didn't do a
search for it properly (I was looking for setf tar). Anyway, the reason
for doing the setting is that the plugin is re-using Bram's tar file
syntax highlighting. I've now changed that so it does a "setf zip"
followed by a "run! syntax/tar.vim" to get the proper highlighting. The
other issue is the important one and its what started misbehaving after
I updated vim.

I've got to keep current on vim so as to keep syntax/vim.vim at least
somewhat current (amongst other reasons).

Thanks!
Chip Campbell

[Christian Brabandt]

Okay, so how about the following patch. It checks, whether the filename starts with something that looks like a protocol specifier like http:/ or in this case zipfile:/ and in that case does not prepend the current directory to it (assuming a plugin will take care of reading this):

diff --git a/src/os_unix.c b/src/os_unix.c
index d192b6bcf..09e1f7d2f 100644
--- a/src/os_unix.c
+++ b/src/os_unix.c
@@ -2648,11 +2648,17 @@ mch_FullName(
                    vim_strncpy(buf, fname, p - fname);
                    if (mch_chdir((char *)buf))
                    {
+                       char_u  *s = vim_strchr(fname, ':');
+
                        // Path does not exist (yet).  For a full path fail,
                        // will use the path as-is.  For a relative path use
                        // the current directory and append the file name.
                        if (mch_isFullName(fname))
                            retval = FAIL;
+                       // the filename begins with a protocol identifier like http://
+                       // do not prepend the path to it, a plugin may handle it
+                       else if ( s != NULL && *++s == '/')
+                           retval = FAIL;
                        else
                            p = NULL;
                    }

[Shane-XB-Qian]

FYI:
the different between tar and zip:

• tarfile::bar.txt
• ~/zipfile:/home/shane/bar.zip::bar.txt

[Charles Campbell]

Christian Brabandt (Vim Github Repository) wrote:
>

Looks like this patch works for me!

Thank you, Christian,

Chip Campbell

[Christian Brabandt]

tarfile::bar.txt

Shoot. That indicates that the tarplugin is also broken with my patch.

[Shane-XB-Qian]

um.. actually tar.vim is working still somehow for now, but zip.vim is not.
// ~/zipfile: that ~/ looks odd to me as first expression.

[lacygoill]

I think the issue could be fixed by passing expand("<amatch>") to fnamemodify(":."). As a suggestion, here is a patch:

diff --git a/runtime/plugin/zipPlugin.vim b/runtime/plugin/zipPlugin.vim
index b9e334fb9..ce9ba0db9 100644
--- a/runtime/plugin/zipPlugin.vim
+++ b/runtime/plugin/zipPlugin.vim
@@ -34,19 +34,19 @@ endif
 " Public Interface: {{{1
 augroup zip
  au!
- au BufReadCmd   zipfile:*	call zip#Read(expand("<amatch>"), 1)
- au FileReadCmd  zipfile:*	call zip#Read(expand("<amatch>"), 0)
- au BufWriteCmd  zipfile:*	call zip#Write(expand("<amatch>"))
- au FileWriteCmd zipfile:*	call zip#Write(expand("<amatch>"))
+ au BufReadCmd   zipfile:*	call expand('<amatch>')->fnamemodify(':.')->zip#Read(1)
+ au FileReadCmd  zipfile:*	call expand('<amatch>')->fnamemodify(':.')->zip#Read(0)
+ au BufWriteCmd  zipfile:*	call expand('<amatch>')->fnamemodify(':.')->zip#Write()
+ au FileWriteCmd zipfile:*	call expand('<amatch>')->fnamemodify(':.')->zip#Write()
 
- if has("unix")
-  au BufReadCmd   zipfile:*/*	call zip#Read(expand("<amatch>"), 1)
-  au FileReadCmd  zipfile:*/*	call zip#Read(expand("<amatch>"), 0)
-  au BufWriteCmd  zipfile:*/*	call zip#Write(expand("<amatch>"))
-  au FileWriteCmd zipfile:*/*	call zip#Write(expand("<amatch>"))
+ if has('unix')
+  au BufReadCmd   zipfile:*/*	call expand('<amatch>')->fnamemodify(':.')->zip#Read(1)
+  au FileReadCmd  zipfile:*/*	call expand('<amatch>')->fnamemodify(':.')->zip#Read(0)
+  au BufWriteCmd  zipfile:*/*	call expand('<amatch>')->fnamemodify(':.')->zip#Write()
+  au FileWriteCmd zipfile:*/*	call expand('<amatch>')->fnamemodify(':.')->zip#Write()
  endif
 
- exe "au BufReadCmd ".g:zipPlugin_ext.' call zip#Browse(expand("<amatch>"))'
+ exe 'au BufReadCmd ' .. g:zipPlugin_ext .. ' call expand("<amatch>")->fnamemodify(":.")->zip#Browse()'
 augroup END
 
 " ---------------------------------------------------------------------

[Shane-XB-Qian]

#8903
#9023

[itchyny]

Considering Vim already deals with some-protocol:// in absolute path calculation (grep with path_with_url), I think replacing zipfile: by zipfile:// (both autoload and plugin) fixes the problem. Can anyone test this?

[依云]

Yes, I did a sed -i 's=zipfile:=zipfile://=g' to all runtime files and it worked.

[itchyny]

Okay, thanks. Unfortunate that fixing a bug broke an existing plugin, but using zipfile:// looks better solution than proposed in this thread. The zip plugin implementation is valid under the idea of URI, where //authority is optional, but Vim cannot distinguish this resource identifier from actual (relative) file paths. Based on the implementation of path_with_url (and vim_isAbsName), using :// in plugins looks a good convention to me.

[itchyny]

@cecamp @chrisbra @lacygoill Do you have any thoughts?

[Christian Brabandt]

I agree, that using zipfile:// make sense.

[cecamp]

When I use zipfile:// as the leader for various zipped file containers, the internally contained files often show up as zipfile:/// . So, would prepending zipfile:/ be sufficient to prevent problems with the patch? (that shows up as zipfile://).

[itchyny]

No, that would not work on Windows because absolute paths start with a drive letter.

[依云]

On Fri, Oct 22, 2021 at 07:00:53AM -0700, cecamp wrote:
> When I use zipfile:// as the leader for various zipped file containers, the internally contained files often show up as zipfile:/// . So, would prepending zipfile:/ be sufficient to prevent problems with the patch? (that shows up as zipfile://).

That is expected to me. The third / indicates that it's an absolute path, just like file:///usr/share/... that browsers use.

--
Best regards,
lilydjwg

[cecamp]

OK, please try v32c of http://www.drchip.org/astronaut/vim/index#ZIP.

btw I knew that browsers use the https:///... style, I just didn't want zip.vim interfering with something -- like browsers.

[cecamp]

Looks like markdown is biting me today. That link should be zip.vim. I hope.

[itchyny]

I think the comment in history section is inconsistent with the changes.

[lacygoill]

Fixed in the latest update of the runtime files.

[K.Takata]

Closed #8974.