terminology tweak: prune -> path limiting
PATCH 1/1 terminology tweak: prune -> path limiting
In the codebase, "prune" is a highly overloaded term, and it caused me a lot of trouble to figure out what it meant when it was used in the context of path limiting. Stop using the word "prune" when we really mean "path limiting."
--- Documentation/technical/api-history-graph.txt | 5 +- builtin/add.c | 4 +- builtin/diff.c | 8 +- builtin/fast-export.c | 2 +- builtin/log.c | 2 +- builtin/rev-list.c | 2 +- diff-lib.c | 6 +- revision.c | 90 ++++++++++--------- revision.h | 4 +- t/t7811-grep-open.sh | 2 +- tree-walk.c | 10 +-- wt-status.c | 4 +- 12 files changed, 72 insertions(+), 67 deletions(-)
Documentation/technical/api-history-graph.txt +3 -2
@@ -100,8 +100,9 @@ Limitations on all parents of that commit. Parents must not be skipped, or the graph output will appear incorrect. + -`graph_update()` may be used on a pruned set of commits only if the parent list -has been rewritten so as to include only ancestors from the pruned set. +`graph_update()` may be used on a pruned (e.g. path-limited) set of commits only +if the parent list has been rewritten so as to include only ancestors from the +pruned set. * The graph API does not currently support reverse commit ordering. In order to implement reverse ordering, the graphing API needs an
builtin/add.c +2 -2
@@ -113,14 +113,14 @@ int add_files_to_cache(const char *prefix,repo_init_revisions (the_repository, &rev, prefix);setup_revisions (0 ,NULL , &rev,NULL );if (pathspec) -copy_pathspec (&rev.prune_data, pathspec); +copy_pathspec (&rev.path_limits, pathspec); rev.diffopt.output_format =DIFF_FORMAT_CALLBACK ; rev.diffopt.format_callback = update_callback; rev.diffopt.format_callback_data = &data; rev.diffopt.flags.override_submodule_config =1 ; rev.max_count =0 ;/* do not compare unmerged paths with stage #2 */ run_diff_files (&rev,DIFF_RACY_IS_MODIFIED ); -clear_pathspec (&rev.prune_data); +clear_pathspec (&rev.path_limits);return !!data.add_errors; }
builtin/diff.c +4 -4
@@ -74,8 +74,8 @@ static int builtin_diff_b_f(struct rev_info *revs,if (argc >1 )usage (builtin_diff_usage); -GUARD_PATHSPEC (&revs->prune_data,PATHSPEC_FROMTOP |PATHSPEC_LITERAL ); - path = revs->prune_data.items[0 ].match; +GUARD_PATHSPEC (&revs->path_limits,PATHSPEC_FROMTOP |PATHSPEC_LITERAL ); + path = revs->path_limits.items[0 ].match;if (lstat (path, &st))die_errno (_ ("failed to stat '%s'" ), path);
@@ -421,8 +421,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)die (_ ("unhandled object '%s' given." ), name); } } -if (rev.prune_data.nr) - paths += rev.prune_data.nr; +if (rev.path_limits.nr) + paths += rev.path_limits.nr;/* * Now, do the arguments look reasonable?
builtin/fast-export.c +1 -1
@@ -1143,7 +1143,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)import_marks (import_filename); lastimportid = last_idnum; -if (import_filename && revs.prune_data.nr) +if (import_filename && revs.path_limits.nr) full_tree =1 ;get_tags_and_duplicates (&revs.cmdline);
builtin/log.c +1 -1
@@ -690,7 +690,7 @@ static void log_setup_revisions_tweak(struct rev_info *rev, struct setup_revision_opt *opt) {if (rev->diffopt.flags.default_follow_renames && - rev->prune_data.nr ==1 ) + rev->path_limits.nr ==1 ) rev->diffopt.flags.follow_renames =1 ;/* Turn --cc/-c into -p --cc/-c when -p was not given */
builtin/rev-list.c +1 -1
@@ -517,7 +517,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)if (show_progress) progress =start_delayed_progress (show_progress,0 ); -if (use_bitmap_index && !revs.prune) { +if (use_bitmap_index && !revs.path_limiting) {if (revs.count && !revs.left_right && !revs.cherry_mark) {uint32_t commit_count;int max_count = revs.max_count;
diff-lib.c +3 -3
@@ -110,7 +110,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)if (diff_can_quit_early (&revs->diffopt))break ; -if (!ce_path_match (istate, ce, &revs->prune_data,NULL )) +if (!ce_path_match (istate, ce, &revs->path_limits,NULL ))continue ;if (ce_stage (ce)) {
@@ -477,7 +477,7 @@ static int oneway_diff(const struct cache_entry * const *src,if (ce_path_match (revs->diffopt.repo->index, idx ? idx : tree, - &revs->prune_data,NULL )) { + &revs->path_limits,NULL )) {do_oneway_diff (o, idx, tree);if (diff_can_quit_early (&revs->diffopt)) { o->exiting_early =1 ;
@@ -543,7 +543,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)struct rev_info revs;repo_init_revisions (opt->repo, &revs,NULL ); -copy_pathspec (&revs.prune_data, &opt->pathspec); +copy_pathspec (&revs.path_limits, &opt->pathspec); revs.diffopt = *opt;if (diff_cache (&revs, tree_oid,NULL ,1 ))
revision.c +47 -43
225 lines not on this page · raw message
revision.h +2 -2
@@ -87,7 +87,7 @@ struct rev_info {/* Basic information */ const char *prefix;const char *def; -struct pathspec prune_data; +struct pathspec path_limits;/* * Whether the arguments parsed by setup_revisions() included any
@@ -111,7 +111,7 @@ struct rev_info {/* Traversal flags */ unsigned int dense:1, - prune:1, + path_limiting:1, no_walk:2, remove_empty_trees:1, simplify_history:1,
t/t7811-grep-open.sh +1 -1
@@ -23,7 +23,7 @@ enum grep_pat_token { test_commit add-user revision.c " } if (seen_dashdash) - read_pathspec_from_stdin(revs, &sb, prune); + read_pathspec_from_stdin(revs, &sb, limiting_paths); strbuf_release(&sb); }
tree-walk.c +5 -5
@@ -365,10 +365,10 @@ static void free_extended_entry(struct tree_desc_x *t) } } -static inline int prune_traversal (struct name_entry *e, -struct traverse_info *info, -struct strbuf *base, -int still_interesting) +static inline int path_limit_traversal (struct name_entry *e, +struct traverse_info *info, +struct strbuf *base, +int still_interesting) {if (!info->pathspec || still_interesting ==2 )return 2 ;
@@ -461,7 +461,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info) }if (!mask)break ; - interesting =prune_traversal (e, info, &base, interesting); + interesting =path_limit_traversal (e, info, &base, interesting);if (interesting <0 )break ;if (interesting) {
wt-status.c +2 -2
@@ -603,7 +603,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s) rev.diffopt.detect_rename = s->detect_rename >=0 ? s->detect_rename : rev.diffopt.detect_rename; rev.diffopt.rename_limit = s->rename_limit >=0 ? s->rename_limit : rev.diffopt.rename_limit; rev.diffopt.rename_score = s->rename_score >=0 ? s->rename_score : rev.diffopt.rename_score; -copy_pathspec (&rev.prune_data, &s->pathspec); +copy_pathspec (&rev.path_limits, &s->pathspec);run_diff_files (&rev,0 ); }
@@ -639,7 +639,7 @@ static void wt_status_collect_changes_index(struct wt_status *s) rev.diffopt.detect_rename = s->detect_rename >=0 ? s->detect_rename : rev.diffopt.detect_rename; rev.diffopt.rename_limit = s->rename_limit >=0 ? s->rename_limit : rev.diffopt.rename_limit; rev.diffopt.rename_score = s->rename_score >=0 ? s->rename_score : rev.diffopt.rename_score; -copy_pathspec (&rev.prune_data, &s->pathspec); +copy_pathspec (&rev.path_limits, &s->pathspec);run_diff_index (&rev,1 ); }
Signature · 1 line
2.17.1
Quoted 4 lines · Matthew DeVore
In the codebase, "prune" is a highly overloaded term, and it caused me a lot of trouble to figure out what it meant when it was used in the context of path limiting. Stop using the word "prune" when we really mean "path limiting."
path limiting is also used for two purposes. "pruning", which is to cull the side branches that do not contribute the changes made to the paths we are interested in, and showing only the changes to the paths that match pathspec.
AFAIK, "prune" is also used to describe unreachable loose objects, but that use is fairly isolated and have little risk of being confusing too much. Are there other uses to make you consider it "highly overloaded"?
My gut feeling is that the result is not reducing "overloading" in a meaningful way, and this change is not worth the churn, but it depends on the answer to the above question.
Thanks.
Quoted 1 line · Junio C Hamano
AFAIK, "prune" is also used to describe unreachable loose objects,
s/describe/& the act of culling/
Quoted 3 lines
but that use is fairly isolated and have little risk of being confusing too much. Are there other uses to make you consider it "highly overloaded"?
Quoted 8 lines · Junio C Hamano
Quoted 4 lines · Matthew DeVore
In the codebase, "prune" is a highly overloaded term, and it caused me a lot of trouble to figure out what it meant when it was used in the context of path limiting. Stop using the word "prune" when we really mean "path limiting."
Quoted 4 lines
path limiting is also used for two purposes. "pruning", which is to cull the side branches that do not contribute the changes made to the paths we are interested in, and showing only the changes to the paths that match pathspec.
Thank you for the clarification re: side branches.
Quoted 5 lines
AFAIK, "prune" is also used to describe unreachable loose objects, but that use is fairly isolated and have little risk of being confusing too much. Are there other uses to make you consider it "highly overloaded"?
This is what I found:
git prune - cull unreachable loose objects
git fetch --prune - remove remote-tracking refs that no longer exist at source. Also note "--prune-tags" option
git notes prune - remove notes for non-existing/unreachable objects git worktree prune - prunes "administrative" files
git prune-packed - removes loose objects that are also in pack files git filter-branch --prune-empty - removes commits that become empty as a result of rewriting.
It seems there are three general categories of the use of the term -
- to remove things from a view (e.g. in path limiting)
- to remove loose objects for efficiency (git prune, git prune-packed)
- to remove other things for either efficiency or to reduce cognitive
overhead (git worktree prune, git fetch --prune)
... and each of these categories has 2+ subcategories.
When I tried to figure out what "prune" and "prune_data" ("data" is quite vague, so these two fields read like "prune_1" and "prune_2") referred to in "revision.h", I basically did "grep -rn prune" and looked through the results, but there were too many uses of the term "prune" to pinpoint its meaning. If I used an IDE I might have been able to "find usages" of struct revs's prune field, and I probably *should have* done that, but I didn't have an IDE prepared and I figured it wasn't important. Then an hour or so later I realized I was still being confused by this term, and set out to figuring out what it actually meant.
Quoted 6 lines
My gut feeling is that the result is not reducing "overloading" in a meaningful way, and this change is not worth the churn, but it depends on the answer to the above question.
Thanks.
Quoted 3 lines · Matthew DeVore
When I tried to figure out what "prune" and "
prune_data" ("data" is quite vague, so these two fields read like "prune_1" and "prune_2") referred to in "revision.h",...
It was unfortunate that 8efdc326 ("rev-lib: Make it easy to do rename tracking (take 2)", 2006-03-10) had to rename "paths" to "prune_data" when attempting to generalize the history simplification mechanism by introducing "prune_fn", which can be used instead of a fixed function, and as one half of the pair, "data used by the prune function" made some sense.
When we partially reverted that over-generalization with 53b2c823 ("revision walker: mini clean-up", 2007-11-05), we probably should have changed the prune_data back to paths, but we didn't. Worse yet, when we made prune_data from a void pointer to struct pathspec at afe069d1 ("struct rev_info: convert prune_data to struct pathspec", 2010-12-17), we really should have renamed it back to paths, but we didn't.
So I think it is a good idea to get rid of prune_data and make it clear it is no longer a generic thing but cannot be anything but a pathspec. I am not sure what the bit should be called, though. It is a bit to enable any history simplification and not limited to pathspec limiting (e.g. simplify-by-decoration enables it, too).
Quoted 7 lines · Junio C Hamano
So I think it is a good idea to get rid of
prune_dataand make it clear it is no longer a generic thing but cannot be anything but a pathspec. I am not sure what the bit should be called, though. It is a bit to enable any history simplification and not limited to pathspec limiting (e.g. simplify-by-decoration enables it, too).
How about the below patch? I used "can_ignore_commits" as the bit name, which - as the commit msg states - reflects the terminology used in get_commit_action and related functions.
Subject: [PATCH] terminology tweak: prune -> path limiting
In the codebase, "prune" is a highly overloaded term, and it caused me a lot of trouble to figure out what it meant when it was used in the context of path limiting and stripping commits from history.
Rename two identifiers: prune_data (which used to be a void* argument to a function called prune_fn) to path_limits, which correctly describes its current use; and prune to can_ignore_commits, which describes well what it means and parallels the terminology used in the get_commit_action function.
--- Documentation/technical/api-history-graph.txt | 5 +- builtin/add.c | 4 +- builtin/diff.c | 8 +- builtin/fast-export.c | 2 +- builtin/log.c | 2 +- builtin/rev-list.c | 2 +- diff-lib.c | 6 +- revision.c | 92 ++++++++++--------- revision.h | 4 +- t/t7811-grep-open.sh | 2 +- tree-walk.c | 10 +- wt-status.c | 4 +- 12 files changed, 73 insertions(+), 68 deletions(-)
Documentation/technical/api-history-graph.txt +3 -2
@@ -100,8 +100,9 @@ Limitations on all parents of that commit. Parents must not be skipped, or the graph output will appear incorrect. + -`graph_update()` may be used on a pruned set of commits only if the parent list -has been rewritten so as to include only ancestors from the pruned set. +`graph_update()` may be used on a pruned (e.g. path-limited) set of commits only +if the parent list has been rewritten so as to include only ancestors from the +pruned set. * The graph API does not currently support reverse commit ordering. In order to implement reverse ordering, the graphing API needs an
builtin/add.c +2 -2
@@ -113,14 +113,14 @@ int add_files_to_cache(const char *prefix,repo_init_revisions (the_repository, &rev, prefix);setup_revisions (0 ,NULL , &rev,NULL );if (pathspec) -copy_pathspec (&rev.prune_data, pathspec); +copy_pathspec (&rev.path_limits, pathspec); rev.diffopt.output_format =DIFF_FORMAT_CALLBACK ; rev.diffopt.format_callback = update_callback; rev.diffopt.format_callback_data = &data; rev.diffopt.flags.override_submodule_config =1 ; rev.max_count =0 ;/* do not compare unmerged paths with stage #2 */ run_diff_files (&rev,DIFF_RACY_IS_MODIFIED ); -clear_pathspec (&rev.prune_data); +clear_pathspec (&rev.path_limits);return !!data.add_errors; }
builtin/diff.c +4 -4
@@ -74,8 +74,8 @@ static int builtin_diff_b_f(struct rev_info *revs,if (argc >1 )usage (builtin_diff_usage); -GUARD_PATHSPEC (&revs->prune_data,PATHSPEC_FROMTOP |PATHSPEC_LITERAL ); - path = revs->prune_data.items[0 ].match; +GUARD_PATHSPEC (&revs->path_limits,PATHSPEC_FROMTOP |PATHSPEC_LITERAL ); + path = revs->path_limits.items[0 ].match;if (lstat (path, &st))die_errno (_ ("failed to stat '%s'" ), path);
@@ -421,8 +421,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)die (_ ("unhandled object '%s' given." ), name); } } -if (rev.prune_data.nr) - paths += rev.prune_data.nr; +if (rev.path_limits.nr) + paths += rev.path_limits.nr;/* * Now, do the arguments look reasonable?
builtin/fast-export.c +1 -1
@@ -1143,7 +1143,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)import_marks (import_filename); lastimportid = last_idnum; -if (import_filename && revs.prune_data.nr) +if (import_filename && revs.path_limits.nr) full_tree =1 ;get_tags_and_duplicates (&revs.cmdline);
builtin/log.c +1 -1
@@ -690,7 +690,7 @@ static void log_setup_revisions_tweak(struct rev_info *rev, struct setup_revision_opt *opt) {if (rev->diffopt.flags.default_follow_renames && - rev->prune_data.nr ==1 ) + rev->path_limits.nr ==1 ) rev->diffopt.flags.follow_renames =1 ;/* Turn --cc/-c into -p --cc/-c when -p was not given */
builtin/rev-list.c +1 -1
@@ -517,7 +517,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)if (show_progress) progress =start_delayed_progress (show_progress,0 ); -if (use_bitmap_index && !revs.prune) { +if (use_bitmap_index && !revs.can_ignore_commits) {if (revs.count && !revs.left_right && !revs.cherry_mark) {uint32_t commit_count;int max_count = revs.max_count;
diff-lib.c +3 -3
@@ -110,7 +110,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)if (diff_can_quit_early (&revs->diffopt))break ; -if (!ce_path_match (istate, ce, &revs->prune_data,NULL )) +if (!ce_path_match (istate, ce, &revs->path_limits,NULL ))continue ;if (ce_stage (ce)) {
@@ -477,7 +477,7 @@ static int oneway_diff(const struct cache_entry * const *src,if (ce_path_match (revs->diffopt.repo->index, idx ? idx : tree, - &revs->prune_data,NULL )) { + &revs->path_limits,NULL )) {do_oneway_diff (o, idx, tree);if (diff_can_quit_early (&revs->diffopt)) { o->exiting_early =1 ;
@@ -543,7 +543,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)struct rev_info revs;repo_init_revisions (opt->repo, &revs,NULL ); -copy_pathspec (&revs.prune_data, &opt->pathspec); +copy_pathspec (&revs.path_limits, &opt->pathspec); revs.diffopt = *opt;if (diff_cache (&revs, tree_oid,NULL ,1 ))
revision.c +48 -44
228 lines not on this page · raw message
revision.h +2 -2
@@ -87,7 +87,7 @@ struct rev_info {/* Basic information */ const char *prefix;const char *def; -struct pathspec prune_data; +struct pathspec path_limits;/* * Whether the arguments parsed by setup_revisions() included any
@@ -111,7 +111,7 @@ struct rev_info {/* Traversal flags */ unsigned int dense:1, - prune:1, + can_ignore_commits:1, no_walk:2, remove_empty_trees:1, simplify_history:1,
t/t7811-grep-open.sh +1 -1
@@ -23,7 +23,7 @@ enum grep_pat_token { test_commit add-user revision.c " } if (seen_dashdash) - read_pathspec_from_stdin(revs, &sb, prune); + read_pathspec_from_stdin(revs, &sb, limiting_paths); strbuf_release(&sb); }
tree-walk.c +5 -5
@@ -365,10 +365,10 @@ static void free_extended_entry(struct tree_desc_x *t) } } -static inline int prune_traversal (struct name_entry *e, -struct traverse_info *info, -struct strbuf *base, -int still_interesting) +static inline int path_limit_traversal (struct name_entry *e, +struct traverse_info *info, +struct strbuf *base, +int still_interesting) {if (!info->pathspec || still_interesting ==2 )return 2 ;
@@ -461,7 +461,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info) }if (!mask)break ; - interesting =prune_traversal (e, info, &base, interesting); + interesting =path_limit_traversal (e, info, &base, interesting);if (interesting <0 )break ;if (interesting) {
wt-status.c +2 -2
@@ -603,7 +603,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s) rev.diffopt.detect_rename = s->detect_rename >=0 ? s->detect_rename : rev.diffopt.detect_rename; rev.diffopt.rename_limit = s->rename_limit >=0 ? s->rename_limit : rev.diffopt.rename_limit; rev.diffopt.rename_score = s->rename_score >=0 ? s->rename_score : rev.diffopt.rename_score; -copy_pathspec (&rev.prune_data, &s->pathspec); +copy_pathspec (&rev.path_limits, &s->pathspec);run_diff_files (&rev,0 ); }
@@ -639,7 +639,7 @@ static void wt_status_collect_changes_index(struct wt_status *s) rev.diffopt.detect_rename = s->detect_rename >=0 ? s->detect_rename : rev.diffopt.detect_rename; rev.diffopt.rename_limit = s->rename_limit >=0 ? s->rename_limit : rev.diffopt.rename_limit; rev.diffopt.rename_score = s->rename_score >=0 ? s->rename_score : rev.diffopt.rename_score; -copy_pathspec (&rev.prune_data, &s->pathspec); +copy_pathspec (&rev.path_limits, &s->pathspec);run_diff_index (&rev,1 ); }
Signature · 1 line
2.17.1