Skip to content

Prune excluded directories from the index walk#741

Open
ehsash wants to merge 3 commits into
zk-org:devfrom
ehsash:fix/exclude-skip-directories
Open

Prune excluded directories from the index walk#741
ehsash wants to merge 3 commits into
zk-org:devfrom
ehsash:fix/exclude-skip-directories

Conversation

@ehsash

@ehsash ehsash commented Jun 23, 2026

Copy link
Copy Markdown

Summary

Fixes #728.

Exclude globs are currently only matched against files, so an excluded
directory is still walked and filtered file by file. On notebooks that contain
large ignored trees (asset directories, vendored repos, ...) the indexer spends
most of its time lstat-ing files it then discards, which makes indexing — and
every zk invocation that reindexes — slow.

This makes the walker prune a directory matched by an exclude glob, with
filepath.SkipDir, instead of descending into it.

Changes

  • shouldIgnorePath now takes an isDir bool:
    • a directory skips the note-extension check (it has no .md extension) and
      is matched against the exclude globs alone;
    • both the bare foo and the subtree foo/** forms prune the directory.
  • paths.Walk prunes a directory rejected by shouldIgnorePath with
    filepath.SkipDir (the notebook root is never pruned).
  • Backward compatible: file-only globs (*.png, dir/*) match exactly as
    before; only directory matches now prune.

Tests

  • internal/util/paths/walk_test.go: a new test asserts the walker never even
    queries paths inside a pruned directory — proving the subtree is skipped,
    not filtered file by file.
  • tests/issue-728.tesh: end-to-end test covering both the foo and foo/**
    forms.
  • gofmt clean; go test ./... passes.

Impact

On a ~12k-note notebook sitting next to a large (40+ GB, ~170k-entry) asset
directory, excluding that directory takes indexing from re-walking every asset
entry on each index to skipping the tree entirely.

Exclude globs were only matched against files, so an excluded directory
was still traversed and filtered file by file. On notebooks that contain
large ignored trees (e.g. asset or vendored directories) this made
indexing needlessly slow.

shouldIgnorePath now takes an isDir flag: for a directory it skips the
note-extension check and matches the exclude globs (supporting both the
"foo" and "foo/**" forms), and Walk prunes a matching directory with
filepath.SkipDir instead of descending into it. File-only globs keep
their previous behaviour.

Adds a unit test asserting the walker never queries paths inside a pruned
directory, plus a tesh end-to-end test.

Closes zk-org#728
Comment thread internal/core/note_index.go Outdated
Comment thread internal/util/paths/walk.go Outdated
@WhyNotHugo

Copy link
Copy Markdown
Contributor

Other than those two details, looks good.

@tjex

tjex commented Jun 26, 2026

Copy link
Copy Markdown
Member

@ehsash Added one change to docs. The inner workings of how a walk is happening doesn't need to be explained in user facing documentation.

Also, it was written that a directory can be excluded with assets (now changed to your-directory for more clarity).

Shouldn't it be your-directory/ to ignore the directory, and not your-directory? Is this how this fix is working, and the documentation missed this detail? Otherwise, what if there is a file with the same name as an excluded directory?

@WhyNotHugo

Copy link
Copy Markdown
Contributor

Otherwise, what if there is a file with the same name as an excluded directory?

If you have a file named assets and a directory named assets, the file doesn't have the proper extension, and would get ignored anyway.

If you have a directory called assets.md a file with the same name, configured extension .md and are ignoring assets.md, then yes, both get ignored.

@tjex

tjex commented Jul 2, 2026

Copy link
Copy Markdown
Member

Ah yep, of course. That makes sense.

@ehsash Not sure if you've seen, but the tesh case is failing. Should I look at it, or you good to look at it?

@ehsash

ehsash commented Jul 2, 2026

Copy link
Copy Markdown
Author

Ah yep, of course. That makes sense.

@ehsash Not sure if you've seen, but the tesh case is failing. Should I look at it, or you good to look at it?

I'll have a look tomorrow afternoon and take the other remarks in account. You'll have a new commit in the PR that fixes this :-)

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.

exclude patterns should skip directories and not only files

3 participants