justinmk / vim-dirvish

Directory viewer for Vim :zap:
Other
1.18k stars 64 forks source link

`x` and `q` don't work with `$ git ls-files | vim +'setf dirvish' -` #119

Closed lacygoill closed 6 years ago

lacygoill commented 6 years ago

Here's a neat example given in the README of the project:

git ls-files | vim +'setf dirvish' -

It works well, except for the q and x buffer-local mappings. They don't work immediately because their rhs are <plug> mappings, which are are not defined in a plugin/ directory, but in an autoload/ one.

Consider this minimal vimrc written in /tmp/vimrc:

set rtp^=~/.vim/plugged/vim-dirvish/
set rtp+=~/.vim/plugged/vim-dirvish/after
filetype plugin indent on

From the directory of vim-dirvish, if I start Vim like this:

$ git ls-files | vim -Nu /tmp/vimrc +'setf dirvish' -

Vim opens a dirvish buffer listing the contents of the directory. But if I press x on a filepath to add it to the arglist, nothing happens.

This PR tries to fix the issue by moving the <plug> mappings in the plugin/ directory.

I think you moved the <plug> mappings in autoload/ on purpose, because doing so allows you to make the set_args() and buf_close() functions local to the script. Thus, you avoid polluting the function namespace with two additional public functions which are not intended to be invoked directly by the user.

I value the reliability more than the reduced noise, but I understand your choice, because the mappings work correctly once you press another mapping whose rhs is defined in plugin/ and which triggers the sourcing of the autoload/ directory. Still, pressing a key which sometimes work, but sometimes don't, is a little distracting because when it doesn't and you forgot the issue, you may lose time in a useless debugging or wondering whether you did something wrong.

Feel free to close the PR if you disagree.


Edit:

By the way, usually dirvish sets 'buftype' to nofile, but not in the previous example. So, when you press Enter on a file to edit it, you may get the error:

dirvish: E37: No write since last change

Maybe it could be slightly improved by setting the option explicitly from the shell command-line:

$ git ls-files | vim +'setf dirvish' +'setl bt=nofile' -
#                                     ^^^^^^^^^^^^^^^^

Edit2:

There may be other settings which are not properly set in the previous example; namely 'swapfile' and 'conceallevel'.

It seems that usually, they are set by s:buf_init(), which is called by a chain of functions calls such as:

dirvish#open() → s:open_dir() → s:buf_init()

Maybe dirvish#open() should be manually invoked from the shell (with a + argument) to handle the settings, but I don't know exactly how.


Edit3:

This shell command seems to fix all the issues:

$ git ls-files | vim +'call dirvish#open("")' -

So, I close the PR.

justinmk commented 6 years ago

this PR tries to fix the issue by moving the <plug> mappings in the plugin/ directory.

Maybe we can instead force autoload , e.g. by calling dirvish#open() at the end of ftplugin/dirvish.vim:

" Force autoload if `ft=dirvish`
:if !exists('*dirvish#open')|try|call dirvish#open()|catch|endtry|endif

By the way, usually dirvish sets 'buftype' to nofile, ... There may be other settings which are not properly set in the previous example; namely 'swapfile' and 'conceallevel'.

For buffers that dirvish does not own, set filetype=dirvish is only meant to provide a subset of useful functionality. Setting buftype=nofile is potentially destructive. Also the conceal stuff won't make sense because the buffer could contain arbitrary content.

lacygoill commented 6 years ago

Ok, I followed your recommendation, revert the old change, and forced the autoload at the end of ftplugin/.

But now, when I execute:

$ git ls-files | vim -Nu /tmp/vimrc +'setf dirvish' -

It raises the E37 error (No write since last change).

I'm going to try to understand the reason, and fix it if I can.

justinmk commented 6 years ago

E37 goes away if you :set nohidden. I don't know exactly what causes it, but it's harmless.

q won't work still, because Dirvish refuses to delete a buffer it doesn't own (b:dirvish is not defined). That's a safety feature.

x works now.