Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support collapse a single folder under cursor #2847

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

linepi
Copy link

@linepi linepi commented Jul 22, 2024

When I use api.tree.expand_all to expand a folder, sometimes I need to collapse a single folder rather than all folders(invoke by api.tree.collapse_all). This feature(api.tree.collapse) can be understood as the inverse operation of api.tree.expand_all.

@linepi
Copy link
Author

linepi commented Jul 22, 2024

There might be some imperfections in the naming, documentation, and code details. What I am presenting here is just a rough idea. If this pull request idea is accepted, I will continue to work hard to make more reasonable, reliable, and complete revisions.

@evertonse
Copy link
Collaborator

When I expand a folder, sometimes I need to collapse a single folder rather than all folders. This can be understood as the inverse operation of expanding.

I think I migh not understand what you mean.
Is this api.node.navigate.parent_close not what you want?

@linepi
Copy link
Author

linepi commented Jul 24, 2024

@evertonse No, that is not the same. api.node.navigate.parent_close will close the parent folder of where cursor is. The new feature api.tree.collapse will close folder and all it subfolder recursively.

@evertonse
Copy link
Collaborator

@evertonse No, that is not the same. api.node.navigate.parent_close will close the parent folder of where cursor is. The new feature api.tree.collapse will close folder and all it subfolder recursively.

Ah I see now, then there should be api.tree.collapse. Have at it

@linepi
Copy link
Author

linepi commented Jul 26, 2024

I update my commit, including the command, doc and explanation. Please review.

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for not seeing this one earlier...

This is great, we should have had this long ago. I'm thinking about a free default keybinding we can use.

We will do this this however:

  • It's missing the keep_buffers parameter of collapse_all
  • collapse.lua repeats collapse-all.lua taking a node instead of using explorer

Please an optional node parameter to collapse-all.fn so that it may perform both and keep buffers.

@@ -1770,6 +1778,9 @@ tree.collapse_all({keep_buffers}) *nvim-tree-api.tree.collapse_all()*
Parameters: ~
• {keep_buffers} (boolean) do not collapse nodes with open buffers.

tree.collapse() *nvim-tree-api.tree.collapse()*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tree.collapse() *nvim-tree-api.tree.collapse()*
tree.collapse() *nvim-tree-api.tree.collapse()*

spacing

@alex-courtis
Copy link
Member

This is great, many thanks for pushing this one.

@linepi
Copy link
Author

linepi commented Jul 28, 2024

Please an optional node parameter to collapse-all.fn so that it may perform both and keep buffers.

All right, I've added and option to collapse_all.fn and removed collapse. Now it's only one function. Please review.

@alex-courtis
Copy link
Member

alex-courtis commented Aug 3, 2024

On :NvimTreeCollapseKeepBuffers

Error executing Lua callback: ...pi/lua/nvim-tree/actions/tree/modifiers/collapse-all.lua:29: attempt to index local 'opts' (a boolean value)
stack traceback:
        ...pi/lua/nvim-tree/actions/tree/modifiers/collapse-all.lua:29: in function 'collapse_all'
        ...site/pack/packer/start/linepi/lua/nvim-tree/commands.lua:145: in function <...site/pack/packer/start/linepi/lua/nvim-tree/commands.lua:144>

It looks like we're changing API from a boolean to an opts table. We'll need to provide backwards compatibility for legacy arguments, similar to tree.toggle. That's a massive pain in the arse and adds unnecessary complexity.

Alternative: add an API method tree.collapse, leaving tree.collapse_all unchanged. They both then call the one collapse function satisfying point 2 in #2847 (review)

TL;DR point 2 was asking to use a single collapse function rather than a single API.

@alex-courtis
Copy link
Member

alex-courtis commented Aug 3, 2024

tree.collapse_all({keep_buffers})          *nvim-tree-api.tree.collapse_all()*
    Collapse the entire tree.

    Parameters: ~
      • {keep_buffers} (boolean) do not collapse nodes with open buffers.

tree.collapse({opts})                          *nvim-tree-api.tree.collapse()*
    Collapse the tree.

    Parameters: ~{opts} (table) optional parameters

    Options: ~
      • {under_cursor} (boolean) only collapse the node under cursor,
                                 default false
      • {keep_buffers} (boolean) do not collapse nodes with open buffers,
                                 default false

This preserves backward compatibility and the API can stay pure, something like:

Api.tree.collapse = wrap(actions.tree.modifiers.collapse_all.collapse)
Api.tree.collapse_all = wrap(actions.tree.modifiers.collapse_all.collapse_all)

collapse_all.fn would be renamed to collapse_all.collapse

collapse_all.collapse_all would then look something like:

---@param keep_buffers boolean
function M.fn(keep_buffers)
  M.collapse({keep_buffers = keep_buffers, under_cursor = false})
end

Finally, rename collapse-all.lua to collapse.lua

Copy link
Member

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, we are close...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants