justinmk / vim-dirvish

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

Fails to select current file if > 1000 files on initial open #172

Closed wincent closed 4 years ago

wincent commented 4 years ago

Repro

mkdir demo
cd demo
for F in $(seq -w 1001); do touch $F.txt; done
vim 0500.txt

ie. open vim in a directory with > 1000 files and open a file somewhere in the middle.

Now hit the - binding for vim-dirvish.

Expected behavior

Actual behavior

Variations

Analysis

Not super familiar with the code base, but this is what seems to be happening:

So, I don't really know how all this is supposed to work. But the repeat call to dirvish#open() is why we see the (probably unwanted) "showing cached listing" message.

If I hard-code s:should_reload() to return 1, I don't see the "cached listing" message, and the appropriate file (0500.txt) gets selected in the view. Likewise if I increase the size limit above 1000 (or if I delete some files to get below the 1000-file limit).

Although this repro describes the initial open, I see similar behavior for subsequent opens too. For example, if I open another file with :e 0700.txt and then hit -, whatever file was previously selected in Dirvish will still be selected. (For the same reason, if I open a file using Dirvish — ie not with :e but by hitting <CR> when the file is selected in Dirvish — then it continues to be selected when I hit -).

Not sure what the right solution is here. A quick hack is to raise the hard-coded limit. Another idea would be to make it configurable via variable. You might also consider just always reloading, as it seems quite fast (and correct is better than fast). On the other hand, the fact that it is calling dirvish#open() twice in a row due to the autocmd sounds like a bit of a smell, but I am not sure how to fix that.

justinmk commented 4 years ago

Thanks for debugging this!

You might also consider just always reloading, as it seems quite fast (and correct is better than fast).

yeah I suppose so. The thought was that it could be unpleasant when navigating around, to constantly reload a listing from the fs. Though I suppose the fs cache should take care of that....

On the other hand, the fact that it is calling dirvish#open() twice in a row due to the autocmd sounds like a bit of a smell

should be harmless unless it's reloading the whole listing, but I don't think that's the case. #open() is the entrypoint and has multiple "modes". This is how Dirvish reuses buffers (and all the state implied there , including cursor position)

wincent commented 4 years ago

Thanks for fixing @justinmk!