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: ungrouping empty directories #2647

Merged
merged 5 commits into from
Feb 4, 2024

Conversation

juefeiyan
Copy link
Contributor

implement #2646

change: if users enabled group_empty, there would be three states when users 'expand_or_collapse' a grouped folders:

1: closed grouped folder:
image

2: opened grouped folder:
image

3: opened ungrouped folder:
image

that feature would allow users to add files/folders in the middle grouped empty folder.

@alex-courtis
Copy link
Member

alex-courtis commented Jan 29, 2024

This works:

20240129_111356

20240129_111252

20240129_111345

20240129_111238

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.

This is a change in behaviour that will interrupt current users. Also, group empty has historically been problematic and we need to reduce the blast radius.

Options:

  1. Add a new option, defaulting off
  2. Add API to specifically map a key to "expand the empties"

2 is preferable as it offers the greatest flexibility and ease of use.

@juefeiyan are you up to the challenge of 2?

return node.nodes
end

---Group empty folders
Copy link
Member

Choose a reason for hiding this comment

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

Group->ungroup and longer description please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will resolve the comments and working on option 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just pushed a new version and implemented option 2, please take a look if you are free

@juefeiyan juefeiyan force-pushed the feat/ungroup-empty-folder branch from d036b06 to ac0e540 Compare January 30, 2024 06:42
@alex-courtis
Copy link
Member

A test case like mkdir a/b/C functions as expected when the grouping is on and off.

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.

This looks great! It turned out better than I thought. I look forward to using it.

  • toggle_group_empty
  • NOP on files
  • mapping name and update-help.sh

@@ -1848,6 +1849,9 @@ node.open.vertical() *nvim-tree-api.node.open.vertical()*
node.open.horizontal() *nvim-tree-api.node.open.horizontal()*
|nvim-tree-api.node.edit()|, file will be opened in a new horizontal split.

node.open.toggle_grouped_folders() *nvim-tree-api.node.open.toggle_grouped_folders()*
Copy link
Member

Choose a reason for hiding this comment

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

This is actually specific - it groups/ungroups one folder and somewhat persists that, which is desirable.

Perhaps:

                                *nvim-tree-api.node.open.toggle_group_empty()*
node.open.toggle_group_empty()
    Toggle |nvim-tree.renderer.group_empty| for a specific folder.
    Does nothing on files.
    Needs |nvim-tree.renderer.group_empty| set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the nice name

lua/nvim-tree/api.lua Show resolved Hide resolved
@@ -2196,6 +2200,7 @@ You are encouraged to copy these to your own |nvim-tree.on_attach| function.
vim.keymap.set('n', 'I', api.tree.toggle_gitignore_filter, opts('Toggle Filter: Git Ignore'))
vim.keymap.set('n', 'J', api.node.navigate.sibling.last, opts('Last Sibling'))
vim.keymap.set('n', 'K', api.node.navigate.sibling.first, opts('First Sibling'))
vim.keymap.set('n', 'L', api.node.open.toggle_grouped_folders,opts('Toggle Grouped Folders'))
Copy link
Member

@alex-courtis alex-courtis Feb 3, 2024

Choose a reason for hiding this comment

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

Suggested change
vim.keymap.set('n', 'L', api.node.open.toggle_grouped_folders,opts('Toggle Grouped Folders'))
vim.keymap.set('n', 'L', api.node.open.toggle_grouped_folders,opts('Toggle Group Empty'))

L seems like the only viable mapping.

@juefeiyan
Copy link
Contributor Author

juefeiyan commented Feb 3, 2024

This looks great! It turned out better than I thought. I look forward to using it.

  • toggle_group_empty
  • NOP on files
  • mapping name and update-help.sh

Hi @alex-courtis , I jsut solved your comments, please take a look at my new MR

@alex-courtis
Copy link
Member

This is looking great - files are indeed a NOP.

Sorry to be a pain; I have one more request:
Do not open folders, only toggle grouping. This function is very specifically a toggle.

I don't know if this is practical - please advise.

@juefeiyan
Copy link
Contributor Author

This is looking great - files are indeed a NOP.

Sorry to be a pain; I have one more request: Do not open folders, only toggle grouping. This function is very specifically a toggle.

I don't know if this is practical - please advise.

great suggestion, I just implement it, please take a look

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.

Works nicely. It's truly a toggle now and functions correctly even for never-opened folders.

Fair warning: group empty is historically fragile. I'll call on you if there's an unforseen issue and we may have to revert depending on the severity.

@alex-courtis alex-courtis merged commit 8cbb1db into nvim-tree:master Feb 4, 2024
4 checks passed
@juefeiyan juefeiyan mentioned this pull request Feb 4, 2024
@przepompownia
Copy link
Contributor

Thank you! I think I asked for a similar feature a long time ago, but I can't find any issue or discussion about it at the moment.

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