Skip to content

fix(#1740): set open state only on folders #1741

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

Merged
merged 1 commit into from
Nov 13, 2022

Conversation

alfa07
Copy link
Contributor

@alfa07 alfa07 commented Nov 11, 2022

Fixes issue 1740

If we set node.open indiscriminately (e.g. for files) then we fail later on this line when we try to draw the tree.

First we will recurse here on a file:
https://github.com/nvim-tree/nvim-tree.lua/blob/master/lua/nvim-tree/renderer/builder.lua#L263-L267

And then tree.node will be nil here:
https://github.com/nvim-tree/nvim-tree.lua/blob/master/lua/nvim-tree/renderer/builder.lua#L285

@alfa07 alfa07 changed the title fix(collaps-all): set open state only on folders fix(#1740): set open state only on folders Nov 11, 2022
@alfa07
Copy link
Contributor Author

alfa07 commented Nov 11, 2022

@alex-courtis @kyazdani42 do you think this is the right way to fix the issue? Another option is to fix the check of node.open in here to is_folder and node.open.

@alex-courtis
Copy link
Member

@alex-courtis @kyazdani42 do you think this is the right way to fix the issue? Another option is to fix the check of node.open in here to is_folder and node.open.

This fixes the root cause, which is the incorrect setting of node.open.

Let's not change the builder itself as there may be other assumptions around open and nodes.

@alex-courtis
Copy link
Member

alex-courtis commented Nov 12, 2022

Fix tested OK however it did reveal an issue with the matcher: no-keep folders that begin with the path of a keep folder are left open:

:; mkdir 1740
:; cd 1740
:; mkdir dir dir1 dir2 dir3
:; echo 0 > dir/file0
:; echo 1 > dir1/file1
:; echo 2 > dir2/file2

asciicast

If you could also resolve that in this PR I would be most grateful @alfa07

Edit: in a follow up PR

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.

Tested OK

@alex-courtis alex-courtis merged commit cf90837 into nvim-tree:master Nov 13, 2022
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.

Error while running :NvimTreeCollapseKeepBuffers
2 participants