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

fix(#2468): always apply filters to subdirectories #2537

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

Akmadan23
Copy link
Collaborator

@Akmadan23 Akmadan23 commented Nov 18, 2023

Closes #2468
I might have found a fix for #2468, and if so it's a really simple one. From my tests it seems to apply correctly all filters to all subdirs, not sure about the resource leaks but they should go together as well.

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.

Good catch!

This looks correct and will now correctly handle closed refreshes. However... we're now watching .git/* directories. We'll need to somehow apply the exclusions that are present elsewhere.

Testing branch vs master:

: ; diff --unified=1000 <(sed -e 's/.*watcher] //g' /tmp/ma.log) <(sed -e 's/.*watcher] //g' /tmp/br.log)
--- /proc/self/fd/11	2023-11-19 15:07:11.978193749 +1100
+++ /proc/self/fd/12	2023-11-19 15:07:11.974860405 +1100
@@ -1,33 +1,48 @@
 Watcher:new '/home/alex/src/nvim-tree/r/2468' nil
 Event:new '/home/alex/src/nvim-tree/r/2468'
 Event:start '/home/alex/src/nvim-tree/r/2468'
 git start
 Watcher:new '/home/alex/src/nvim-tree/r/2468/.git' { "FETCH_HEAD", "HEAD", "HEAD.lock", "config", "index" }
 Event:new '/home/alex/src/nvim-tree/r/2468/.git'
 Event:start '/home/alex/src/nvim-tree/r/2468/.git'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/n' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/n'
 Event:start '/home/alex/src/nvim-tree/r/2468/n'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/n/1' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/n/1'
 Event:start '/home/alex/src/nvim-tree/r/2468/n/1'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/n/2' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/n/2'
 Event:start '/home/alex/src/nvim-tree/r/2468/n/2'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/n/3' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/n/3'
 Event:start '/home/alex/src/nvim-tree/r/2468/n/3'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/i' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/i'
 Event:start '/home/alex/src/nvim-tree/r/2468/i'
+Watcher:new '/home/alex/src/nvim-tree/r/2468/.git/branches' nil
+Event:new '/home/alex/src/nvim-tree/r/2468/.git/branches'
+Event:start '/home/alex/src/nvim-tree/r/2468/.git/branches'
+Watcher:new '/home/alex/src/nvim-tree/r/2468/.git/hooks' nil
+Event:new '/home/alex/src/nvim-tree/r/2468/.git/hooks'
+Event:start '/home/alex/src/nvim-tree/r/2468/.git/hooks'
+Watcher:new '/home/alex/src/nvim-tree/r/2468/.git/info' nil
+Event:new '/home/alex/src/nvim-tree/r/2468/.git/info'
+Event:start '/home/alex/src/nvim-tree/r/2468/.git/info'
+Watcher:new '/home/alex/src/nvim-tree/r/2468/.git/objects' nil
+Event:new '/home/alex/src/nvim-tree/r/2468/.git/objects'
+Event:start '/home/alex/src/nvim-tree/r/2468/.git/objects'
+Watcher:new '/home/alex/src/nvim-tree/r/2468/.git/refs' nil
+Event:new '/home/alex/src/nvim-tree/r/2468/.git/refs'
+Event:start '/home/alex/src/nvim-tree/r/2468/.git/refs'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/i/1' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/i/1'
 Event:start '/home/alex/src/nvim-tree/r/2468/i/1'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/i/2' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/i/2'
 Event:start '/home/alex/src/nvim-tree/r/2468/i/2'
 Watcher:new '/home/alex/src/nvim-tree/r/2468/i/3' nil
 Event:new '/home/alex/src/nvim-tree/r/2468/i/3'
 Event:start '/home/alex/src/nvim-tree/r/2468/i/3'
 Watcher:destroy '/home/alex/src/nvim-tree/r/2468/i'
 Event:destroy '/home/alex/src/nvim-tree/r/2468/i'

@Akmadan23
Copy link
Collaborator Author

Not sure what the correct behavior should be, I did some tests and the watchers are triggered for .git subdirs only if .git is visible, which makes sense... What are the exclusions that might be present elsewhere?

@alex-courtis
Copy link
Member

Not sure what the correct behavior should be, I did some tests and the watchers are triggered for .git subdirs only if .git is visible, which makes sense... What are the exclusions that might be present elsewhere?

I see. I think I had some incorrect assumptions around existing behaviour.

The files that trigger a full refresh are the curated list:

-- Files under .git that should result in a reload when changed.
to avoid unwanted noise.

These directories are treated like any other and thus shouldn't result in any performance issues.

I'm happy if you are.

@Akmadan23 Akmadan23 merged commit fb89297 into master Nov 20, 2023
@Akmadan23 Akmadan23 deleted the 2468-fix-subfolders-events branch November 20, 2023 10:43
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.

filtered subfolder Events not torn down
2 participants