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

matchers: refactor RepoPathTree (prep for glob matcher) #3508

Merged
merged 7 commits into from
Apr 16, 2024

Conversation

yuja
Copy link
Contributor

@yuja yuja commented Apr 15, 2024

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

yuja added 7 commits April 15, 2024 21:28
I'm going to extract generic map from RepoPathTree, and .get_visit_sets()
will be inlined into FilesMatcher/PrefixMatcher. These removed tests should
be covered by the corresponding matcher tests.
Perhaps, I didn't do that because it's important to initialize is_dir/file to
false. Since I'm going to extract a generic map-like API, and is_dir/file will
be an enum, this won't be a problem.
…ir flag

The is_dir flag will be removed soon. Since FilesMatcher doesn't set is_dir
flag explicitly, is_dir is equivalent to !entries.is_empty(). OTOH,
PrefixMatcher always sets is_dir, so all tree nodes are directories.
This prepares for adding glob matcher, which will be backed by
RepoPathTree<Vec<glob::Pattern>>.

FilesNodeKind/PrefixNodeKind are basically boolean types, but implemented as
enums for better code readability.
@torquestomp torquestomp self-assigned this Apr 15, 2024
Copy link
Contributor

@torquestomp torquestomp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice change, thanks!

@yuja yuja merged commit 7bed5dd into jj-vcs:main Apr 16, 2024
16 checks passed
@yuja yuja deleted the push-sozkmuvmplkw branch April 16, 2024 01:10
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.

2 participants