fix(#3178): handle Windows paths in ignore_dirs and add .zig-cache to defaults#3261
fix(#3178): handle Windows paths in ignore_dirs and add .zig-cache to defaults#3261
Conversation
There was a problem hiding this comment.
The existing utils.escape_special_characters utility was not suitable for this fix as it only handles parentheses and curly braces in paths, not the fundamental path separator difference between Unix (/) and Windows ().
Good call. There may be unintended side effects.
Tested on Windows 10 with Zig build process.
Many thanks for the detailed testing!
Please:
- revert all unrelated formatting changes
- double escape the backslashes https://github.com/nvim-tree/nvim-tree.lua/pull/3261/changes#r2806727955
lua/nvim-tree/explorer/watch.lua
Outdated
| notify.error( | ||
| string.format( | ||
| "Observed %d consecutive file system events with interval < %dms, exceeding filesystem_watchers.max_events=%s. Disabling watcher for directory '%s'. Consider adding this directory to filesystem_watchers.ignore_dirs", | ||
| watcher.data.outstanding_events, | ||
| M.config.filesystem_watchers.max_events, | ||
| M.config.filesystem_watchers.debounce_delay, | ||
| node.absolute_path | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Please do not make any formatting changes unrelated to this PR.
lua/nvim-tree/explorer/watch.lua
Outdated
| if type(M.config.filesystem_watchers.ignore_dirs) == "table" then | ||
| for _, ignore_dir in ipairs(M.config.filesystem_watchers.ignore_dirs) do | ||
| if utils.is_windows or true then | ||
| ignore_dir = ignore_dir:gsub("/", "\\") or ignore_dir |
There was a problem hiding this comment.
We will need to double escape the backslashes as per testing: #3178 (comment)
|
I've reverted the unrelated formatting (sorry for this one) and double escaped the backslashes I searched about what it would cause and makes sense thanks again for the call. Even after reverting the formatting there still some checks that are not passing even on my local I need to commit with |
lua/nvim-tree/explorer/watch.lua
Outdated
| end | ||
|
|
||
| if type(M.config.filesystem_watchers.ignore_dirs) == "table" then | ||
| print(vim.inspect(M.config.filesystem_watchers.ignore_dirs)) |
There was a problem hiding this comment.
Please remove the print...
There was a problem hiding this comment.
I've reverted the unrelated formatting (sorry for this one) and double escaped the backslashes I searched about what it would cause and makes sense thanks again for the call.
Even after reverting the formatting there still some checks that are not passing even on my local I need to commit with
--no-verify.
I see. It looks like is_folder_ignored is indented with tabs, not spaces. You can format just that file with :lua vim.lsp.buf.format() or gg=G
You have identified a bigger problem: style-check does not identify all the issue that style-fix does. Issue incoming... #3271
Please:
- format
watch.lua - remove the print statement
|
All good to go Sir Alex |
There was a problem hiding this comment.
Getting close...
Please:
- Add zig-cache to defaults https://github.com/nvim-tree/nvim-tree.lua/pull/3261/changes#r2819702785
- Resolve regression https://github.com/nvim-tree/nvim-tree.lua/pull/3261/changes#r2819704632
| "/build", | ||
| "/node_modules", | ||
| "/target", | ||
| "/.zig-cache", |
There was a problem hiding this comment.
This is not present in nvim-tree.lua DEFAULT_OPTS
Please add it and re-generate the help: https://github.com/nvim-tree/nvim-tree.lua/blob/master/CONTRIBUTING.md#help-documentation
lua/nvim-tree/explorer/watch.lua
Outdated
|
|
||
| if type(M.config.filesystem_watchers.ignore_dirs) == "table" then | ||
| for _, ignore_dir in ipairs(M.config.filesystem_watchers.ignore_dirs) do | ||
| if utils.is_windows or true then |
There was a problem hiding this comment.
Should that true be there?
Testing shows a regression:
mkdir foo
mkdir .ccls-cacheSet filters.dotfiles and git_ignored false
Open tree and directories
touch foo/1Expected: 1 appears immediately.
touch .ccls-cache/2Expected: 2 does not appear until R
Actual: appears immediately
There was a problem hiding this comment.
This was a testing I was doing my bad, I should have removed it
|
@alex-courtis I've opened 3 PR so far and some issues, where 2 PR's refactors and one bug fix (very simple and with clear hints). How would you rate my contributions from 0-10 so far, only rate if possible, maybe you lack some data I don't know. |
|
Tested OK: mkdir foo
mkdir .ccls-cache
mkdir .zig-cacheSet touch foo/1Expected: 1 appears immediately. touch .ccls-cache/2Expected: 2 does not appear until R touch .zig-cache/3Expected: 3 does not appear until R |
alex-courtis
left a comment
There was a problem hiding this comment.
Tested OK: #3261 (comment) on linux and you have tested windows.
Please accept my deepest apologies, I missed this one: we need to update the defaults in lua/nvim-tree/_meta/config/filesystem_watchers.lua and generate help.
How do you like to handle these tiny nits?
- Fix it yourself
- I push a fix
No drama either way, everyone has their preference.
|
Done. |
There was a problem hiding this comment.
Love your work @Uanela
Edit: apologies, approved too early.
We're getting a CI fail on the help txt for the new default: https://github.com/nvim-tree/nvim-tree.lua/actions/runs/22170271960/job/64106472696?pr=3261
Please:
- Regenerate help to include the recent change https://github.com/nvim-tree/nvim-tree.lua/blob/master/CONTRIBUTING.md#help-documentation
|
Docs regeneration done. I got the problem that wasn't allowing me to regenerate the docs: /tmp/src/neovim-stable/src/gen/gen_vimdoc.lua.org -> /tmp/src/neovim-stable/src/gen/gen_vimdoc.lua
scripts/help-defaults.sh
sed: -e: No such file or directory
make: *** [help-update] Error 1
uanela_como@Uanelas-MacBook-Pro nvim-tree.lua % sudo make help-updatethe |
This PR fixes a critical memory leak issue on Windows where
nvim-tree.luaspawns infinite filesystem watchers when the.zig-cache/tmpfolder creates temporary files during a build process. The watcher locks these files, preventing Zig from deleting them and causing memory to grow by 10-20 MB/second.Changes
Added
.zig-cacheto defaultignore_dirs- This directory contains build artifacts not under version control, similar to existing defaults likenode_modulesandbuild.Fixed Windows path handling in
ignore_dirs- The forward slash prefix (e.g.,/node_modules) used in default ignore patterns wasn't matching Windows paths (e.g.,M:\path\to\node_modules). The fix converts forward slashes to backslashes on Windows to ensure proper pattern matching.Technical Details
The existing
utils.escape_special_charactersutility was not suitable for this fix as it only handles parentheses and curly braces in paths, not the fundamental path separator difference between Unix (/) and Windows (\).The solution implements OS-specific path normalization in
is_folder_ignored()by converting forward slashes to backslashes on Windows systems before pattern matching withvim.fn.match().Testing
Tested on Windows 10 with Zig build process. The
.zig-cachedirectory is now properly ignored, and existing defaults likenode_modulesnow correctly match on Windows paths.Closes #3178