Implement a RepoPathTreeIterator #2406
Conversation
torquestomp
force-pushed
the
repo-path-tree
branch
from
aad282c to
19fea5a
Compare
|
Why do we want this? It doesn't seem to be used anywhere. Could you describe what it's for in the commit message because currently this seems to just introduce dead code? I think the idea is to use it in custom binaries. Do you think it's going to be common enough that we should carry the code in the repo instead of just having it in the Google customization? Or can it not be implemented without access to internals? (I haven't looked at the code yet.) |
torquestomp
force-pushed
the
repo-path-tree
branch
from
19fea5a to
935399c
Compare
|
I updated the commit description. I haven't looked through the internal code to see where this could be used (it might be useful somewhere), but yes the main intent is opening up the lib a little more to extensions. It seems like a natural extension of the type (which has private internals), so I think it would be nice to leave in the public API. An extension could implement this on their own of course as a copy but it wouldn't make any use of jj_lib. |
torquestomp
force-pushed
the
repo-path-tree
branch
from
935399c to
32844c8
Compare
The internal 'set-of-dirs' structure is useful to extensions that need to do their own tree-walking routines. It may also be useful internally in other places.
torquestomp
force-pushed
the
repo-path-tree
branch
from
32844c8 to
db3d40c
Compare
There was a problem hiding this comment.
Some further context: Google wants to calculate the set of trees that changed in each commit so we can store that in a central index and use it for prefetching trees from the server later. We already index the set of files changed in a commit. We're thinking of calculating the set of changed directories by taking that set of files and building up a RepoPathTree from them, then looking up the tree id for each of those directories in the commit's MergedTree (we only care about the "after" state).
I wonder if we should instead teach MergedTree's diff iterator an option to also include the trees it visits. That would also be useful for the TODO in jj debug tree (which I (incidentally!) added just a few days ago). That seems cleaner because we avoid looking up the trees from the backend again after the diff iterator had just done it. It won't matter in practice, though, because the trees will be cached already at that point.
@torquestomp: If we just make the type and a few existing methods public (maybe just new(), add_dir(), and add_file()?), maybe that's all we need if we then walk the tree by using the Matcher::visit() method?
I hesitate to merge this PR because I dislike having unused code in the codebase, even though I agree that it seems plausible that we'll want it in the future. I'd at least want hear a non-Googlers opinion before we merge it.
There was a problem hiding this comment.
Delete.
To make *~ files always ignored, you can do this:
$ git config --global core.excludesFile ~/.gitignore
$ echo '*~' >> ~/.gitignore
(you can call the file .gitignore file whatever you want)
| let map_component = *self.map_component.as_ref().unwrap(); | ||
| child.path.push_front(map_component); | ||
| return Some(child); | ||
| } |
There was a problem hiding this comment.
nit: set self.tree_iter = None here so we don't visit the subdirectory for later files in this tree
Checklist
If applicable:
CHANGELOG.md