mcchrish / nnn.vim

File manager for vim/neovim powered by n³
BSD 2-Clause "Simplified" License
646 stars 25 forks source link

Add file explorer #123

Closed N-R-K closed 3 years ago

N-R-K commented 3 years ago

Completes https://github.com/mcchrish/nnn.vim/pull/103, rebased on top of master.

Nothing has been documented yet however. Also most of this is trail and error and looking at current code, so more testing might be needed.

Aside from the following, no existing code is touched. (I'm not quite sure why this is needed) @mizlan @mcchrish Can you confirm if this is needed or not? Can just revert if not. Reverted this as it doesn't seem to be needed.

@@ -287,6 +351,36 @@ function! nnn#pick(...) abort

     let l:opts.term = s:build_window(l:layout, { 'cmd': l:cmd, 'on_exit': s:create_on_exit_callback(l:opts) })
     let b:tbuf = l:opts.term.buf
+    autocmd BufEnter <buffer> startinsert
     setfiletype nnn
     if g:nnn#statusline && !s:present(l:layout, 'window')
N-R-K commented 3 years ago

One problem right now is that opening a Picker while Explorer is active causes the Picker to behave very badly.

N-R-K commented 3 years ago

Here's a preview: https://asciinema.org/a/ooDoQMQMUTNgU8JEj5yZbC8Ni

Explorer works fine, and Picker also works fine. The problem at the end only occurs when Picker is invoked while Explorer is active.

luukvbaal commented 3 years ago

The problem at the end only occurs when Picker is invoked while Explorer is active.

This is why the original PR got stuck isn't it? @mizlan intended to do the refactor required for multiple instances but never got around to it.

N-R-K commented 3 years ago

This is why the original PR got stuck isn't it?

Maybe, but multiple instances of NnnPicker seems to work fine... An ad-hoc solution might be to disable NnnPicker when NnnExplorer is active.

There's still the "problem" of code duplication and the 0.5sec sleep waiting for FIFO to be created. Nothing fatal, can be cleaned up later.

N-R-K commented 3 years ago

Hmm, the problem seems to be caused just by opening NnnExplorer once. Picker behaves badly even after closing Explorer.

mcchrish commented 3 years ago

The problem at the end only occurs when Picker is invoked while Explorer is active.

This is why the original PR got stuck isn't it? @mizlan intended to do the refactor required for multiple instances but never got around to it.

I think a lot of this is caused by shared variables between the instances, mainly around the script-local vars s:. One way to refactor this is perhaps convert the variables to be buffer-local b: so that each nnn buffer instances are self-contained. Might take a look at functions like setbufvar, getbufvar.

N-R-K commented 3 years ago

Seems like the problem wasn't caused by the shared variables, but rather due to setting NNN_FIFO. Added a commit which no longer tries to modify NNN_FIFO directly. So far everything seems to be working fine.

jarun commented 3 years ago

Cool!

@luukvbaal can you please have a look?

luukvbaal commented 3 years ago

You need to filter out the a flag from NNN_OPTS in the plugin or it will not work for people who have that set. in lua:

local nnnopts = os.getenv("NNN_OPTS")
local exploreropts = nnnopts and nnnopts:gsub("a", "") or ""

and pass NNN_OPTS = exploreropts to explorer cmd.

Otherwise it seems to work(not tested thorougly). I would suggest atleast setlocal nonumber norelativenumber noshowmode in the explorer buffer. I set some others as well in nnn.nvim.

N-R-K commented 3 years ago

I would suggest atleast setlocal nonumber norelativenumber noshowmode in the explorer buffer.

ftplugin/nnn.vim has some of these set

https://github.com/mcchrish/nnn.vim/blob/f7ebbaa41da15a964758f009e8e05463974a0aee/ftplugin/nnn.vim#L10

You need to filter out the a flag from NNN_OPTS in the plugin or it will not work for people who have that set.

Good catch, I'll look into that.

P.S: fixed in https://github.com/mcchrish/nnn.vim/pull/123/commits/607c334c258738dadea97d8186ad07714af817dc

luukvbaal commented 3 years ago

P.S: fixed in 607c334

I was talking about shell env NNN_OPTS not g:nnn#command. I'm still seeing the same issue because I have a in my NNN_OPTS. My g:nnn#command is empty.

In nnn.nvim I filter it out programatically from NNN_OPTS and mention in docs that you shouldn't set the -a flag for in explorer cmd cfg.

N-R-K commented 3 years ago

@luukvbaal I see, can you test out https://github.com/mcchrish/nnn.vim/pull/123/commits/8518665f96fd8aca08a3281f363bd869337e0c52 and confirm if that solves the issue?

Also out of curiosity, where do you set your $NNN_OPTS ? If it was on shellrc then I don't think it would've affected nnn instances run via nnn.vim.

luukvbaal commented 3 years ago

@luukvbaal I see, can you test out 8518665 and confirm if that solves the issue?

Explorer works now yeah, be aware that currently running NnnPicker while the explorer window is active will open the file in the explorer window, and vice versa.

Also out of curiosity, where do you set your $NNN_OPTS ? If it was on shellrc then I don't think it would've affected nnn instances run via nnn.vim.

In my .zprofile.

jarun commented 3 years ago

@N-R-K

be aware that currently running NnnPicker while the explorer window is active will open the file in the explorer window, and vice versa.

Can we document this?

luukvbaal commented 3 years ago

Or fix it by filtering out the picker/explorer window for the target window. It's what I did in nnn.nvim atleast.

N-R-K commented 3 years ago

I'll look into it. @jarun

Might have to touch how we're exiting/closing windows as well. Exiting explorer behaves a bit weird when picker is invoked, prints something like "process exited with N" instead of closing the window.

N-R-K commented 3 years ago

currently running NnnPicker while the explorer window is active will open the file in the explorer window

Can you elaborate on this or show some recording? I don't think I understand the problem here. @luukvbaal

luukvbaal commented 3 years ago

Hero you go, the gif shows a secondary bug as well that happens when closing the explorer after running NnnPicker, resulting in an error. asciicast

mcchrish commented 3 years ago

Some points that might help solve the issue:

The previous window/location is remember when calling nnn#picker. This is the ppos opt. Since the previous location is the explorer window, it's where the selected file is opened.

Solution is instead of remembering the explorer window id, just remember the window beside it (or above/below depending on the config). You can probably use something like winnr('1h') or winnr('1j') etc. then pass it to win_getid.

N-R-K commented 3 years ago

@luukvbaal Ahh, I see now, can reproduce. Not sure how common workflow it is to open the Picker from the explorer window, but I'll look into this regardless.

a secondary bug as well that happens when closing the explorer after running NnnPicker

Yes, I mentioned it here https://github.com/mcchrish/nnn.vim/pull/123#issuecomment-944500528

Might have to touch how we're exiting/closing windows as well. Exiting explorer behaves a bit weird when picker is invoked, prints something like "process exited with N" instead of closing the window.

@mcchrish Thanks for the hints. I don't think I'll be able to look at this today, but hopefully I'll have some time for this tomorrow.

mizlan commented 3 years ago

Hello, sorry for disappearing.

I can no longer read the code I had once written... :(

Has there been consideration of rewriting this to use the same logic as luukvbaal/nnn.nvim but in Vimscript instead of Lua?

N-R-K commented 3 years ago

@luukvbaal https://github.com/mcchrish/nnn.vim/pull/123/commits/6232710c950e3e8563a6fc545b197b63efe0f3f9 should fix the "secondary bug". Please confirm.

Has there been consideration of rewriting this to use the same logic as luukvbaal/nnn.nvim but in Vimscript instead of Lua?

I'm not sure if that's possible since nnn.nvim uses nvim specific api (i think).

luukvbaal commented 3 years ago

Can confirm that the error is gone in the situation show before.

The same still error shows up when running NnnPicker twice and trying to close the two floating windows that are created, (in my plugin running the command while it is already open toggles the window visibility instead of opening a second instance hence why I ran into it, not sure if the opposite is documented/expected behavior here).

N-R-K commented 3 years ago

The same still error shows up when running NnnPicker twice and trying to close the two floating windows that are created

I don't think that's related to this PR/file-explorer. I get the same errors on master.

luukvbaal commented 3 years ago

No I don't think so either, just mentioned it.

jarun commented 3 years ago

@N-R-K @luukvbaal are we good to go?

N-R-K commented 3 years ago

@jarun the PR is functional from what I've tested. So I'd say yes.

Calling :NnnPicker from explorer window feels like a rare workflow, not unwilling to fix it, but that will require me to look deeper into how vim(script) works so that might take abit.

We can merge this PR and then deal with that later. Otherwise if we want to fix it here, then we'll need to wait more until I'm free to take a deeper look on it.

jarun commented 3 years ago

not unwilling to fix it, but that will require me to look deeper into how vim(script) works so that might take abit

Yes, we will deal with the unrelated issue separately. Check it when you get the time.

@mizlan probably you can also take a look into it if you are back.

jarun commented 3 years ago

Thanks guys!

jarun commented 3 years ago

@N-R-K can you add a clean section on how to setup the explorer mode?

N-R-K commented 3 years ago

@jarun sure. Will take a bit, but should be able to do it today.

jarun commented 3 years ago

Thanks!

jarun commented 3 years ago

When I press Enter on a file from explorer, is it supposed to open the file in a new buffer?

N-R-K commented 3 years ago

When I press Enter on a file from explorer, is it supposed to open the file in a new buffer?

I think so, yes. This is the expected behavior https://asciinema.org/a/JhAzVx1ajE2pG1jEwPnundiqs (ignore the visual glitch on the nnn window at the end). Are you experiencing something else?

jarun commented 3 years ago

I was seeing it open in a new buffer because of minibufexplorer plugin. Once I disable that, it's good. I am on vim. Check it out once.

jarun commented 3 years ago

I also see the following msg when I quit vim every time (without minibufexplorer as well):

mkfifo: cannot create fifo '/tmp/v4TBWVj/0': File exists

The use case is:

E947: Job still running in buffer "!/bin/bash"
Press ENTER or type command to continue
jarun commented 3 years ago

I have in .vimrc:

let g:nnn#set_default_mappings = 0
let g:nnn#replace_netrw = 1
nnoremap <leader>n :NnnExplorer %:p:h<CR>
let g:nnn#command = 'nnn -C'
let g:nnn#action = { '<c-h>': 'vsplit' }
let g:nnn#explorer_layout = { 'left': '~20%' }
N-R-K commented 3 years ago

Try to quit with :qa (I see the following msg in red:

Can reproduce. Works fine if the nnn explorer is closed naturally via q first however. Will look into it.

I was seeing it open in a new buffer because of minibufexplorer plugin. Once I disable that, it's good.

The AutoComplPop plugin also behaves badly with this plugin (both on explorer and picker mode) and randomly renames files when switching from/into the nnn buffer. Will look into both these plugin related problems later tomorrow.

jarun commented 3 years ago

Thanks!