feat: add `quit_on_open` and `focus` opts to `api.node.open.edit` #3054

GCrispino · 2025-01-26T15:05:29Z

fixes #1984

As discussed on #1984 , this should implement the ability of specifying the following two options in the api.node.open.edit method:

quit_on_open: if set to true, it closes the tree when file related to the node is opened;
focus: if set to ~~true~~ false, opens the file but keep the focus on the tree. Similar to what's done in the preview feature, but actually creating a new buffer for each file.

For now I've added a draft commit that supports the first option and then just checks it and closes the tree view in the edit API method.
Not sure if that's the best option, but at first I decided to not forward it to the action level to not mix it with the quit_on_open global option that can be passed in to the action, and also to keep the changes restricted to the API code only.

Thoughts?

alex-courtis

Works beautifully:

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.edit(nil, { quit_on_open = true })
  end, opts("Open Quit"))

Handles directories, root etc. as expected.

For now I've added a draft commit that supports the first option and then just checks it and closes the tree view in the edit API method.
Not sure if that's the best option, but at first I decided to not forward it to the action level to not mix it with the quit_on_open global option that can be passed in to the action, and also to keep the changes restricted to the API code only.

I like that approach, it's really clear what's going on. The open_file is far too complex and needs to be split up into file opening and post-actions; this is the first step.

alex-courtis · 2025-01-27T00:15:13Z

lua/nvim-tree/api.lua

 end

 ---@param mode string
 ---@param toggle_group boolean?
---@return fun(node: Node)
+---@return fun(node: Node, edit_opts: NodeEditOpts?)
 local function open_or_expand_or_dir_up(mode, toggle_group)
  ---@param node Node


Missing param, CI got upset: https://github.com/nvim-tree/nvim-tree.lua/actions/runs/12975816599/job/36187330634?pr=3054

Needs

---@param edit_opts NodeEditOpts?

I recommend setting up wiki: Lua Language Server, which will show a warning about these sorts of things.

Ok will do, thanks :)

alex-courtis · 2025-01-27T00:27:16Z

If it helps, you're a member of the project and can now push branches directly, rather than your fork.

I've added "fixes #1984" to the description: that issue will be automatically closed once this is merged.

gegoune · 2025-01-27T01:03:09Z

@GCrispino Hi! 👋 quick bit: instead of adding WIP to subject you can open draft PR which is a way clearer way to communicate.

Congrats on your first PR here. :)

GCrispino · 2025-01-27T01:38:13Z

@alex-courtis

Works beautifully:
  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.edit(nil, { quit_on_open = true })
  end, opts("Open Quit"))
Handles directories, root etc. as expected.

For now I've added a draft commit that supports the first option and then just checks it and closes the tree view in the edit API method.
Not sure if that's the best option, but at first I decided to not forward it to the action level to not mix it with the quit_on_open global option that can be passed in to the action, and also to keep the changes restricted to the API code only.

I like that approach, it's really clear what's going on. The open_file is far too complex and needs to be split up into file opening and post-actions; this is the first step.

Great! Will continue following this then 👍🏼

If it helps, you're a member of the project and can now push branches directly, rather than your fork.

I've added "fixes #1984" to the description: that issue will be automatically closed once this is merged.

Oh yeah, totally forgot about this 😅 . Definitely easier to just push a new branch into the project, will do this on the next one, thanks for the reminder 👍🏼

@gegoune

@GCrispino Hi! 👋 quick bit: instead of adding WIP to subject you can open draft PR which is a way clearer way to communicate.

Oh yeah sorry wasn't aware GitHub had draft PRs - set this one to be draft for now and removed the "WIP" prefix, thanks for the recommendation 👍🏼

Congrats on your first PR here. :)

Thank you!

GCrispino · 2025-01-27T03:09:11Z

I believe this is now done, so removing the draft status.

Something I'd like to note is that, since other methods on api.node (such as api.node.open.vertical and api.node.open.horizontal) wrap the edit function that is modified in this PR, this means that these methods will implicitly gain support for the two new options added here.

Would this be a problem, or if this isn't, would we need to add documentation anywhere for this?

alex-courtis · 2025-01-27T03:20:02Z

this means that these methods will implicitly gain support for the two new options added here.

Wow... they do indeed. We'll need to test them all...

would we need to add documentation anywhere for this?

Proposal:

add {opts} param to each nvim-tree-api.node.open. function in 6.3.
add a note something like this to cover them all

==============================================================================
 6.3 API NODE                                             *nvim-tree-api.node*

    Parameters for all `nvim-tree-api.node` functions: ~
      • {node} (Node|nil) file or folder

    Optional {opts} for all `nvim-tree-api.node.open` functions: ~
      • {force} (boolean) delete even if buffer is modified, default false
      • {quit_on_open} (boolean) ...

alex-courtis · 2025-01-27T03:53:30Z

Testing OK, one failure with .tab

I didn't have the energy to test all open variants...

Can I please leave the drop functions with you?

function M.on_attach(bufnr)
  local function opts(desc)
    return { desc = "nvim-tree: " .. desc, buffer = bufnr, noremap = true, silent = true, nowait = true }
  end

  api.config.mappings.default_on_attach(bufnr)

  vim.keymap.set("n", "?", api.tree.toggle_help, opts("Help"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.edit(nil, {})
  end, opts("Edit"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.edit(nil, { quit_on_open = true, focus = false, })
  end, opts("Edit Quit No Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.edit(nil, { quit_on_open = true, focus = true, })
  end, opts("Edit Quit Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.edit(nil, { quit_on_open = false, focus = false, })
  end, opts("Edit No Focus"))


  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.vertical(nil, {})
  end, opts("Vert"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.vertical(nil, { quit_on_open = true, focus = false, })
  end, opts("Vert Quit No Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.vertical(nil, { quit_on_open = true, focus = true, })
  end, opts("Vert Quit Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.vertical(nil, { quit_on_open = false, focus = false, })
  end, opts("Vert No Focus"))


  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.horizontal_no_picker(nil, {})
  end, opts("Horiz No Picker"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.horizontal_no_picker(nil, { quit_on_open = true, focus = false, })
  end, opts("Horiz No Picker Quit No Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.horizontal_no_picker(nil, { quit_on_open = true, focus = true, })
  end, opts("Horiz No Picker Quit Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.horizontal_no_picker(nil, { quit_on_open = false, focus = false, })
  end, opts("Horiz No Picker No Focus"))


  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.replace_tree_buffer(nil, {})
  end, opts("Replace Tree Buffer"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.replace_tree_buffer(nil, { quit_on_open = true, focus = false, })
  end, opts("Replace Tree Buffer Quit No Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.replace_tree_buffer(nil, { quit_on_open = true, focus = true, })
  end, opts("Replace Tree Buffer Quit Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.replace_tree_buffer(nil, { quit_on_open = false, focus = false, })
  end, opts("Replace Tree Buffer No Focus"))


  -- these do work, even thought they do not make much sense
  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.preview(nil, {})
  end, opts("Preview"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.preview(nil, { quit_on_open = true, focus = false, })
  end, opts("Preview Quit No Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.preview(nil, { quit_on_open = true, focus = true, })
  end, opts("Preview Quit Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.preview(nil, { quit_on_open = false, focus = false, })
  end, opts("Preview No Focus"))


  -- FAIL
  -- tree is not closed, but focus is correct
  -- skip options for this one?
  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.tab(nil, {})
  end, opts("Tab"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.tab(nil, { quit_on_open = true, focus = false, })
  end, opts("Tab Quit No Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.tab(nil, { quit_on_open = true, focus = true, })
  end, opts("Tab Quit Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.tab(nil, { quit_on_open = false, focus = false, })
  end, opts("Tab No Focus"))


  -- these do work, even thought they do not make much sense
  -- this function really should not be inside open
  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.toggle_group_empty(nil, {})
  end, opts("Toggle Group Empty"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.toggle_group_empty(nil, { quit_on_open = true, focus = false, })
  end, opts("Toggle Group Empty Quit No Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.toggle_group_empty(nil, { quit_on_open = true, focus = true, })
  end, opts("Toggle Group Empty Quit Focus"))

  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.toggle_group_empty(nil, { quit_on_open = false, focus = false, })
  end, opts("Toggle Group Empty No Focus"))



end

GCrispino · 2025-01-29T02:27:09Z

Proposal:

add {opts} param to each nvim-tree-api.node.open. function in 6.3. add a note something like this to cover them all

==============================================================================
 6.3 API NODE                                             *nvim-tree-api.node*

    Parameters for all `nvim-tree-api.node` functions: ~
      • {node} (Node|nil) file or folder

    Optional {opts} for all `nvim-tree-api.node.open` functions: ~
      • {force} (boolean) delete even if buffer is modified, default false
      • {quit_on_open} (boolean) ...

I'm fine with this proposal 👍🏼

Testing OK, one failure with .tab

I didn't have the energy to test all open variants...

Can I please leave the drop functions with you?

Yeah sure, I just didn't have the time to give this another look.
I just have two questions regarding this.

On your on_attach function referenced on your latest message for testing the changes in this PR, you have multiple mappings for <S-CR>
1.1. do you comment all of them and leave the one you're trying to test uncommented, or do you have something special to make all of them work?
Has the addition of an automated testing tool/framework been discussed for nvim-tree? I'm a newbie in the nvim world so I don't really know how this is handled generally, but it seems like that would extremely helpful in cases like this one

if tabpage is not nil, then the function closes the tabpage in this specific tabpage

GCrispino · 2025-01-31T01:45:56Z

I just pushed some commits with the fix for the tab functions.

I still need to test the drop functions though

alex-courtis · 2025-02-01T00:51:54Z

1.1. do you comment all of them and leave the one you're trying to test uncommented, or do you have something special to make all of them work?

Sorry, yes, I should have mentioned. Really hacky.

Has the addition of an automated testing tool/framework been discussed for nvim-tree? I'm a newbie in the nvim world so I don't really know how this is handled generally, but it seems like that would extremely helpful in cases like this one

Yes. It is very much needed, we just lack the time/people to select a framework and start building tests.

There's a refactoring task for NodeIterator and I'm honestly too scared to do it without tests.

Edit: you kicked me into action: chore: POC plenary test #3063

alex-courtis · 2025-02-01T01:34:41Z

I just pushed some commits with the fix for the tab functions.

I still need to test the drop functions though

tab works as expected, as does tab_drop.

drop is not working for two cases:

  -- new file is opened, however the previous one remains in the window
  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.drop(nil, { quit_on_open = true, focus = false, })
  end, opts("Drop No Focus"))

  -- new file is opened, however the previous one remains in the window
  vim.keymap.set("n", "<S-CR>", function()
    api.node.open.drop(nil, { quit_on_open = true, focus = true, })
  end, opts("Drop Quit Focus"))

echo foo > foo
echo bar > bar

open nvim
open bar
navigate to foo
api.node.open.drop(nil, { quit_on_open = true, focus = false, })
foo buffer is opened, bar is not dropped and remains in the window

Repeat for api.node.open.drop(nil, { quit_on_open = true, focus = true, })

alex-courtis · 2025-02-01T01:39:42Z

CI is currently failing, see #3060 (comment)

alex-courtis

Looking good!

You accidentally picked an issue that hits one of the most annoying, fragile parts of this codebase node.open_file.fn :)

drop feat: add quit_on_open and focus opts to api.node.open.edit #3054 (comment)
add help feat: add quit_on_open and focus opts to api.node.open.edit #3054 (comment)

alex-courtis · 2025-02-01T01:38:47Z

lua/nvim-tree/api.lua

+  if not focus then
+    -- if mode == "tabnew" a new tab will be opened and we need to focus back to the previous tab
+    if mode == "tabnew" then
+      vim.cmd(":tabprev")


Suggested change

vim.cmd(":tabprev")

vim.cmd.tabprev()

When neovim gets around to exposing these as proper API we will be ready.

GCrispino · 2025-02-15T00:41:49Z

Hey @alex-courtis , apologies for the delay here, but I have been busy with other things in the past few weeks and haven't had the opportunity to test this further until today.

So I did a few tests and couldn't come up with a fix for these scenarios with the api.node.open.drop, but I have some small findings, perhaps you can have a clue on what's going on here:

About the focus functionality on drop:

It seems like when dropping the new nvim-tree window changes or something like that, and a different window is focused instead.

For example, when following the steps below with the foo and bar files created here:

open nvim
open bar
run :NvimTreeOpen
run :lua vim.print(vim.api.nvim_get_current_win()) - in my case it outputted 1002
run :drop foo

foo is opened

navigate back to tree and run :lua vim.print(vim.api.nvim_get_current_win()) again - in my case it outputted 1003

Even when testing with nvim-tree without my modified code, I could reproduce this.

This might be part of my lack of knowledge of vim's inner APIs, but I would expect that the second call to vim.api.nvim_get_current_win() yields 1002 again - or is it expected that the window gets modified?

About the close functionality on drop:

The nvim-tree window is closed indeed, but for some reason it doesn't open the new file (in your example, foo).

I don't know why that's the case, but I could accidentally find out that when putting an vim.fn.input() statement before this line, just to pause execution and try understanding a bit more how this is working, after pressing enter for continuing execution, the foo file was opened as expected 🤔 .

Do you have any idea of why this happened? 😅

alex-courtis · 2025-02-15T04:21:35Z

Hey @alex-courtis , apologies for the delay here, but I have been busy with other things in the past few weeks and haven't had the opportunity to test this further until today.

There is never any pressure or rush; this is a community project with members all having limited availability. My window is also limited - usually Sun/Mon.

So I did a few tests and couldn't come up with a fix for these scenarios with the api.node.open.drop, but I have some small findings, perhaps you can have a clue on what's going on here:

About the focus functionality on drop:

It seems like when dropping the new nvim-tree window changes or something like that, and a different window is focused instead.

For example, when following the steps below with the foo and bar files created here:

open nvim

open bar

run :NvimTreeOpen

run :lua vim.print(vim.api.nvim_get_current_win()) - in my case it outputted 1002

run :drop foo

foo is opened

navigate back to tree and run :lua vim.print(vim.api.nvim_get_current_win()) again - in my case it outputted 1003

Even when testing with nvim-tree without my modified code, I could reproduce this.

This might be part of my lack of knowledge of vim's inner APIs, but I would expect that the second call to vim.api.nvim_get_current_win() yields 1002 again - or is it expected that the window gets modified?

About the close functionality on drop:

The nvim-tree window is closed indeed, but for some reason it doesn't open the new file (in your example, foo).

I don't know why that's the case, but I could accidentally find out that when putting an vim.fn.input() statement before this line, just to pause execution and try understanding a bit more how this is working, after pressing enter for continuing execution, the foo file was opened as expected 🤔 .

I like that trick... I'll definitely be using it in the future.

Do you have any idea of why this happened? 😅

We're in difficult territory here; it's very hard to predict or react to window focus changes that nvim itself is making. Honestly, any workarounds usually end up being buggy and thus we avoid them. Events are sometimes not even sent, when debugging them.

Proposal: limit the scope.

We know which actions are safe and work, and some of the actions like preview do not make any sense.

Limit the scope of this option to known good and desirable cases only.

Proposed actions to remove:

.*drop.*
replace_tree_buffer
preview
toggle_group_empty

Tab is the only special case, but it's rock solid.

We'd need to document the new option for each function in nvim-tree-api.node

Don't worry about api.lua complexity; it's a complete mess and will be overhauled #2896 #2832

feat: add quit_on_open opt to api.node.open.edit

671eedf

alex-courtis reviewed Jan 27, 2025

View reviewed changes

GCrispino marked this pull request as draft January 27, 2025 01:31

GCrispino changed the title ~~WIP: feat: add quit_on_open opt to api.node.open.edit~~ feat: add quit_on_open opt to api.node.open.edit Jan 27, 2025

GCrispino added 3 commits January 26, 2025 23:42

fix: fix missing @param annotation

f40488d

Merge branch 'master' into feat/new-api-edit-opts

f29521c

feat: add focus opt to api.node.open.edit

980a5c1

GCrispino changed the title ~~feat: add quit_on_open opt to api.node.open.edit~~ feat: add quit_on_open and focus opts to api.node.open.edit Jan 27, 2025

GCrispino marked this pull request as ready for review January 27, 2025 03:09

gegoune changed the title ~~feat: add quit_on_open and focus opts to api.node.open.edit~~ feat: add quit_on_open and focus opts to api.node.open.edit Jan 27, 2025

GCrispino added 3 commits January 30, 2025 22:38

fix: fix focus == false behaviour on api.node.open.tab command

505167e

fix: add optional tabpage integer parameter to view.close

0a2f219

if tabpage is not nil, then the function closes the tabpage in this specific tabpage

fix: fix quit_on_open == true behaviour on api.node.open.tab command

f4757a3

alex-courtis requested changes Feb 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `quit_on_open` and `focus` opts to `api.node.open.edit` #3054

feat: add `quit_on_open` and `focus` opts to `api.node.open.edit` #3054

GCrispino commented Jan 26, 2025 •

edited

Loading

alex-courtis left a comment •

edited

Loading

alex-courtis Jan 27, 2025

GCrispino Jan 27, 2025

alex-courtis commented Jan 27, 2025

gegoune commented Jan 27, 2025

GCrispino commented Jan 27, 2025

GCrispino commented Jan 27, 2025

alex-courtis commented Jan 27, 2025 •

edited

Loading

alex-courtis commented Jan 27, 2025

GCrispino commented Jan 29, 2025

GCrispino commented Jan 31, 2025

alex-courtis commented Feb 1, 2025 •

edited

Loading

alex-courtis commented Feb 1, 2025 •

edited

Loading

alex-courtis commented Feb 1, 2025

alex-courtis left a comment

alex-courtis Feb 1, 2025

GCrispino commented Feb 15, 2025

alex-courtis commented Feb 15, 2025

feat: add quit_on_open and focus opts to api.node.open.edit #3054

Are you sure you want to change the base?

feat: add quit_on_open and focus opts to api.node.open.edit #3054

Conversation

GCrispino commented Jan 26, 2025 • edited Loading

alex-courtis left a comment • edited Loading

Choose a reason for hiding this comment

alex-courtis Jan 27, 2025

Choose a reason for hiding this comment

GCrispino Jan 27, 2025

Choose a reason for hiding this comment

alex-courtis commented Jan 27, 2025

gegoune commented Jan 27, 2025

GCrispino commented Jan 27, 2025

GCrispino commented Jan 27, 2025

alex-courtis commented Jan 27, 2025 • edited Loading

alex-courtis commented Jan 27, 2025

GCrispino commented Jan 29, 2025

GCrispino commented Jan 31, 2025

alex-courtis commented Feb 1, 2025 • edited Loading

alex-courtis commented Feb 1, 2025 • edited Loading

alex-courtis commented Feb 1, 2025

alex-courtis left a comment

Choose a reason for hiding this comment

alex-courtis Feb 1, 2025

Choose a reason for hiding this comment

GCrispino commented Feb 15, 2025

alex-courtis commented Feb 15, 2025

Proposal: limit the scope.

feat: add `quit_on_open` and `focus` opts to `api.node.open.edit` #3054

feat: add `quit_on_open` and `focus` opts to `api.node.open.edit` #3054

GCrispino commented Jan 26, 2025 •

edited

Loading

alex-courtis left a comment •

edited

Loading

alex-courtis commented Jan 27, 2025 •

edited

Loading

alex-courtis commented Feb 1, 2025 •

edited

Loading

alex-courtis commented Feb 1, 2025 •

edited

Loading