Skip to content

feat: support collapse a single folder under cursor #2847

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

Closed
wants to merge 1 commit into 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 linepi force-pushed the collapse_folder branch from fa472f2 to 593323f Compare July 26, 2024 14:19
@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 linepi force-pushed the collapse_folder branch from 593323f to 583718a Compare July 28, 2024 06:47
@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...

@alex-courtis
Copy link
Member

Closing as this PR is stale. Please reopen if you wish to continue this work.

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.

3 participants