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.

Sign up for GitHub

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

Jump to bottom

Bug: The local filesystem tree is doubling up on entries #50

Closed
davep opened this issue May 25, 2023 · 3 comments
Closed

Bug: The local filesystem tree is doubling up on entries #50

davep opened this issue May 25, 2023 · 3 comments
Labels
bug Something isn't working Textual Issue Issues that are really down to how Textual works

Comments

@davep
Copy link
Contributor

davep commented May 25, 2023

Mentioned in a video on YouTube. The content of the local file view is doubling up the entries for some reason:

Screenshot 2023-05-25 at 11 03 26
@davep davep added the bug Something isn't working label May 25, 2023
@davep
Copy link
Contributor Author

davep commented May 25, 2023 •
edited
Loading

Weird. My initial suspicion was something relating to the recent changes to DirectoryTree and perhaps the use of a filter; so I removed the filter for a moment, ran up the application, and it fixed the issue. So I put the filer back, ran up the application, and...

Screenshot 2023-05-25 at 12 50 02

So now I can't actually recreate the issue any more!

@davep
Copy link
Contributor Author

davep commented May 25, 2023

Recreating this is a bit hit-and-miss. On my own M2 Pro Mac Mini I can generally recreate this if I run the pipx-installed version, but if I run a "local" copy from the repo I can never reproduce it.

I do, however, have a hypothesis as to what might be going on. The recent changes to DirectoryTree in Textual itself, to load directories in the background, does have a possible window of opportunity whereby the root of the tree could end up getting loaded twice, in an additive way. I'm struggling to think of a way of forcing that to happen, but I think there's a chance it could and it would explain what I'm seeing here.

@davep davep mentioned this issue May 25, 2023
Closed
davep added a commit to davep/textual that referenced this issue May 25, 2023
@davep
Ensure a node has no children before populating it
32790de 
I'm struggling to recreate Textualize/frogmouth#50
in a controlled way, but reviewing the code here makes me think that this is
a good idea anyway. While DirectoryTree should not end up in _populate_node
if a node has already been populated, it's also the case that it's an
all-or-nothing thing; it makes sense to clear out the children of the node
before populating it; at least in a belt-and-braces way.
davep added a commit to davep/textual that referenced this issue May 25, 2023
@davep
Perform the "is loaded" test in _add_to_load_queue
ec3334e 
The test if a node was loaded wasn't being performed when loading the root.
This ensures that will happen. I suspect this is (no pun...) at the root of
the issue with Textualize/frogmouth#50 even though
I can't see the route into how this happens, and can't recreate this at
will.

This feels like a worthwhile change to make anyway as it's a safer approach.
@davep davep mentioned this issue May 25, 2023
Merged
@davep davep added the Textual Issue Issues that are really down to how Textual works label Jun 9, 2023
@willmcgugan
Copy link
Contributor

Assuming fix. Unable to reproduce with latest Textual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working Textual Issue Issues that are really down to how Textual works
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@davep @willmcgugan