Hi, On Thu, 11 Feb 2021, Junio C Hamano wrote: > In the implementation of "git difftool", there is a case where the > user wants to start viewing the diffs at a specific path and > continue on to the rest, optionally wrapping around to the > beginning. Since it is somewhat cumbersome to implement such a > feature as a post-processing step of "git diff" output, let's > support it internally with two new options. > > - "git diff --rotate-to=C", when the resulting patch would show > paths A B C D E without the option, would "rotate" the paths to > shows patch to C D E A B instead. It is an error when there is > no patch for C is shown. > > - "git diff --skip-to=C" would instead "skip" the paths before C, > and shows patch to C D E. Again, it is an error when there is no > patch for C is shown. > > - "git log [-p]" also accepts these two options, but it is not an > error if there is no change to the specified path. Instead, the > set of output paths are rotated or skipped to the specified path > or the first path that sorts after the specified path. This is more a drive-by comment, as I have no stake in this, but don't we already have a more generic solution via `-O<orderfile>`? See https://git-scm.com/docs/git-diff#Documentation/git-diff.txt--Oltorderfilegt for details. Thanks, Dscho > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > > * Please do not take the presense of --skip-to and --rotate-to as > encouraging "git difftool" to gain the same pair of options. I > think "--skip-to" would suffice for the originally presented use > case (i.e. eeek, my difftool session got interrupted when I was > looking at path X, and I only have just a few more paths to go, > X, Y and Z) and if that is the case, there is no need to expose > "--rotate-to" to the end users. Of course, if it is easy to quit > the "difftool" session in the middle, then giving the users only > "--rotate-to" without exposing "--skip-to" is also OK. > > I decided to diagnose a pathname that is not in the diff-queue as > an error when running the "diff" family, while allowing the > command to continue when running the "log" family, so it may turn > out to be that "git difftool --skip-to" do not have to implement > its own error handling after all. > > It has been quite a long time for me to write any real patch with > a non-trivial amount of code change myself, so I may be getting > rusty, but it is refreshing to write code every so often ;-) > > Documentation/diff-options.txt | 8 ++++ > Documentation/gitdiffcore.txt | 21 ++++++++++ > Makefile | 1 + > builtin/diff-files.c | 1 + > builtin/diff-index.c | 2 + > builtin/diff-tree.c | 3 ++ > builtin/diff.c | 1 + > diff.c | 21 ++++++++++ > diff.h | 21 ++++++++++ > diffcore-rotate.c | 46 ++++++++++++++++++++++ > diffcore.h | 1 + > t/t4056-diff-order.sh | 72 +++++++++++++++++++++++++++++++++- > 12 files changed, 197 insertions(+), 1 deletion(-) > create mode 100644 diffcore-rotate.c > > diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt > index e5733ccb2d..7c5b3cf42b 100644 > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -700,6 +700,14 @@ matches a pattern if removing any number of the final pathname > components matches the pattern. For example, the pattern "`foo*bar`" > matches "`fooasdfbar`" and "`foo/bar/baz/asdf`" but not "`foobarx`". > > +--skip-to=<file>:: > +--rotate-to=<file:: > + Discard the files before the named <file> from the output > + (i.e. 'skip to'), or move them to the end of the output > + (i.e. 'rotate to'). These were invented primarily for use > + of the `git difftool` command, and may not be very useful > + otherwise. > + > ifndef::git-format-patch[] > -R:: > Swap two inputs; that is, show differences from index or > diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt > index c970d9fe43..2bd1220477 100644 > --- a/Documentation/gitdiffcore.txt > +++ b/Documentation/gitdiffcore.txt > @@ -74,6 +74,7 @@ into another list. There are currently 5 such transformations: > - diffcore-merge-broken > - diffcore-pickaxe > - diffcore-order > +- diffcore-rotate > > These are applied in sequence. The set of filepairs 'git diff-{asterisk}' > commands find are used as the input to diffcore-break, and > @@ -276,6 +277,26 @@ Documentation > t > ------------------------------------------------ > > +diffcore-rotate: For Changing At Which Path Output Starts > +--------------------------------------------------------- > + > +This transformation takes one pathname, and rotates the set of > +filepairs so that the filepair for the given pathname comes first, > +optionally discarding the paths that come before it. This is used > +to implement the `--skip-to` and the `--rotate-to` options. It is > +an error when the specified pathname is not in the set of filepairs, > +but it is not useful to error out when used with "git log" family of > +commands, because it is unreasonable to expect that a given path > +would be modified by each and every commit shown by the "git log" > +command. For this reason, when used with "git log", the filepair > +that sorts the same as, or the first one that sorts after, the given > +pathname is where the output starts. > + > +Use of this transformation combined with diffcore-order will produce > +unexpected results, as the input to this transformation is likely > +not sorted when diffcore-order is in effect. > + > + > SEE ALSO > -------- > linkgit:git-diff[1], > diff --git a/Makefile b/Makefile > index 5a239cac20..9b1bde2e0e 100644 > --- a/Makefile > +++ b/Makefile > @@ -863,6 +863,7 @@ LIB_OBJS += diffcore-delta.o > LIB_OBJS += diffcore-order.o > LIB_OBJS += diffcore-pickaxe.o > LIB_OBJS += diffcore-rename.o > +LIB_OBJS += diffcore-rotate.o > LIB_OBJS += dir-iterator.o > LIB_OBJS += dir.o > LIB_OBJS += editor.o > diff --git a/builtin/diff-files.c b/builtin/diff-files.c > index 4742a4559b..e037efb07e 100644 > --- a/builtin/diff-files.c > +++ b/builtin/diff-files.c > @@ -54,6 +54,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix) > } > if (!rev.diffopt.output_format) > rev.diffopt.output_format = DIFF_FORMAT_RAW; > + rev.diffopt.rotate_to_strict = 1; > > /* > * Make sure there are NO revision (i.e. pending object) parameter, > diff --git a/builtin/diff-index.c b/builtin/diff-index.c > index 7f5281c461..06635e8fb2 100644 > --- a/builtin/diff-index.c > +++ b/builtin/diff-index.c > @@ -41,6 +41,8 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix) > if (!rev.diffopt.output_format) > rev.diffopt.output_format = DIFF_FORMAT_RAW; > > + rev.diffopt.rotate_to_strict = 1; > + > /* > * Make sure there is one revision (i.e. pending object), > * and there is no revision filtering parameters. > diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c > index 9fc95e959f..b6a9a9328e 100644 > --- a/builtin/diff-tree.c > +++ b/builtin/diff-tree.c > @@ -156,6 +156,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix) > if (merge_base && opt->pending.nr != 2) > die(_("--merge-base only works with two commits")); > > + opt->diffopt.rotate_to_strict = 1; > + > /* > * NOTE! We expect "a..b" to expand to "^a b" but it is > * perfectly valid for revision range parser to yield "b ^a", > @@ -192,6 +194,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix) > int saved_nrl = 0; > int saved_dcctc = 0; > > + opt->diffopt.rotate_to_strict = 0; > if (opt->diffopt.detect_rename) { > if (!the_index.cache) > repo_read_index(the_repository); > diff --git a/builtin/diff.c b/builtin/diff.c > index 5cfe1717e8..f1b88c7389 100644 > --- a/builtin/diff.c > +++ b/builtin/diff.c > @@ -491,6 +491,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) > } > > rev.diffopt.flags.recursive = 1; > + rev.diffopt.rotate_to_strict = 1; > > setup_diff_pager(&rev.diffopt); > > diff --git a/diff.c b/diff.c > index 69e3bc00ed..71e4738548 100644 > --- a/diff.c > +++ b/diff.c > @@ -5348,6 +5348,19 @@ static int diff_opt_word_diff_regex(const struct option *opt, > return 0; > } > > +static int diff_opt_rotate_to(const struct option *opt, const char *arg, int unset) > +{ > + struct diff_options *options = opt->value; > + > + BUG_ON_OPT_NEG(unset); > + if (!strcmp(opt->long_name, "skip-to")) > + options->skip_instead_of_rotate = 1; > + else > + options->skip_instead_of_rotate = 0; > + options->rotate_to = arg; > + return 0; > +} > + > static void prep_parse_options(struct diff_options *options) > { > struct option parseopts[] = { > @@ -5599,6 +5612,12 @@ static void prep_parse_options(struct diff_options *options) > DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG), > OPT_FILENAME('O', NULL, &options->orderfile, > N_("control the order in which files appear in the output")), > + OPT_CALLBACK_F(0, "rotate-to", options, N_("<path>"), > + N_("show the change in the specified path first"), > + PARSE_OPT_NONEG, diff_opt_rotate_to), > + OPT_CALLBACK_F(0, "skip-to", options, N_("<path>"), > + N_("skip the output to the specified path"), > + PARSE_OPT_NONEG, diff_opt_rotate_to), > OPT_CALLBACK_F(0, "find-object", options, N_("<object-id>"), > N_("look for differences that change the number of occurrences of the specified object"), > PARSE_OPT_NONEG, diff_opt_find_object), > @@ -6669,6 +6688,8 @@ void diffcore_std(struct diff_options *options) > diffcore_pickaxe(options); > if (options->orderfile) > diffcore_order(options->orderfile); > + if (options->rotate_to) > + diffcore_rotate(options); > if (!options->found_follow) > /* See try_to_follow_renames() in tree-diff.c */ > diff_resolve_rename_copy(); > diff --git a/diff.h b/diff.h > index 2ff2b1c7f2..45300e3597 100644 > --- a/diff.h > +++ b/diff.h > @@ -227,6 +227,27 @@ enum diff_submodule_format { > struct diff_options { > const char *orderfile; > > + /* > + * "--rotate-to=<file>" would start showing at <file> and when > + * the output reaches the end, wrap around by default. > + * Setting skip_instead_of_rotate to true stops the output at the > + * end, effectively discarding the earlier part of the output > + * before <file>'s diff (this is used to implement the > + * "--skip-to=<file>" option). > + * > + * When rotate_to_strict is set, it is an error if there is no > + * <file> in the diff. Otherwise, the output starts at the > + * path that is the same as, or first path that sorts after, > + * <file>. Because it is unreasonable to require the exact > + * match for "git log -p --rotate-to=<file>" (i.e. not all > + * commit would touch that single <file>), "git log" sets it > + * to false. "git diff" sets it to true to detect an error > + * in the command line option. > + */ > + const char *rotate_to; > + int skip_instead_of_rotate; > + int rotate_to_strict; > + > /** > * A constant string (can and typically does contain newlines to look for > * a block of text, not just a single line) to filter out the filepairs > diff --git a/diffcore-rotate.c b/diffcore-rotate.c > new file mode 100644 > index 0000000000..445f060ab0 > --- /dev/null > +++ b/diffcore-rotate.c > @@ -0,0 +1,46 @@ > +/* > + * Copyright (C) 2021, Google LLC. > + * Based on diffcore-order.c, which is Copyright (C) 2005, Junio C Hamano > + */ > +#include "cache.h" > +#include "diff.h" > +#include "diffcore.h" > + > +void diffcore_rotate(struct diff_options *opt) > +{ > + struct diff_queue_struct *q = &diff_queued_diff; > + struct diff_queue_struct outq; > + int rotate_to, i; > + > + if (!q->nr) > + return; > + > + for (i = 0; i < q->nr; i++) { > + int cmp = strcmp(opt->rotate_to, q->queue[i]->two->path); > + if (!cmp) > + break; /* exact match */ > + if (!opt->rotate_to_strict && cmp < 0) > + break; /* q->queue[i] is now past the target pathname */ > + } > + > + if (q->nr <= i) { > + /* we did not find the specified path */ > + if (opt->rotate_to_strict) > + die(_("No such path '%s' in the diff"), opt->rotate_to); > + return; > + } > + > + DIFF_QUEUE_CLEAR(&outq); > + rotate_to = i; > + > + for (i = rotate_to; i < q->nr; i++) > + diff_q(&outq, q->queue[i]); > + for (i = 0; i < rotate_to; i++) { > + if (opt->skip_instead_of_rotate) > + diff_free_filepair(q->queue[i]); > + else > + diff_q(&outq, q->queue[i]); > + } > + free(q->queue); > + *q = outq; > +} > diff --git a/diffcore.h b/diffcore.h > index d2a63c5c71..c1592bcd01 100644 > --- a/diffcore.h > +++ b/diffcore.h > @@ -164,6 +164,7 @@ void diffcore_rename(struct diff_options *); > void diffcore_merge_broken(void); > void diffcore_pickaxe(struct diff_options *); > void diffcore_order(const char *orderfile); > +void diffcore_rotate(struct diff_options *); > > /* low-level interface to diffcore_order */ > struct obj_order { > diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh > index 63ea7144bb..aec1d9d1b4 100755 > --- a/t/t4056-diff-order.sh > +++ b/t/t4056-diff-order.sh > @@ -1,6 +1,6 @@ > #!/bin/sh > > -test_description='diff order' > +test_description='diff order & rotate' > > GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > @@ -127,4 +127,74 @@ do > ' > done > > +### rotate and skip > + > +test_expect_success 'rotate and skip setup' ' > + >sample1.t && > + >sample2.t && > + >sample3.t && > + >sample4.t && > + git add sample[1234].t && > + git commit -m "added" sample[1234].t && > + echo modified >>sample1.t && > + echo modified >>sample2.t && > + echo modified >>sample4.t && > + git commit -m "updated" sample[1234].t > +' > + > +test_expect_success 'diff --rotate-to' ' > + git diff --rotate-to=sample2.t --name-only HEAD^ >actual && > + test_write_lines sample2.t sample4.t sample1.t >expect && > + test_cmp expect actual > +' > + > +test_expect_success 'diff --skip-to' ' > + git diff --skip-to=sample2.t --name-only HEAD^ >actual && > + test_write_lines sample2.t sample4.t >expect && > + test_cmp expect actual > +' > + > +test_expect_success 'diff --rotate/skip-to error condition' ' > + test_must_fail git diff --rotate-to=sample3.t HEAD^ && > + test_must_fail git diff --skip-to=sample3.t HEAD^ > +' > + > +test_expect_success 'log --rotate-to' ' > + git log --rotate-to=sample3.t --raw HEAD~2.. >raw && > + # just distill the commit header and paths > + sed -n -e "s/^commit.*/commit/p" \ > + -e "/^:/s/^.* //p" raw >actual && > + > + cat >expect <<-\EOF && > + commit > + sample4.t > + sample1.t > + sample2.t > + commit > + sample3.t > + sample4.t > + sample1.t > + sample2.t > + EOF > + > + test_cmp expect actual > +' > + > +test_expect_success 'log --skip-to' ' > + git log --skip-to=sample3.t --raw HEAD~2.. >raw && > + # just distill the commit header and paths > + sed -n -e "s/^commit.*/commit/p" \ > + -e "/^:/s/^.* //p" raw >actual && > + > + cat >expect <<-\EOF && > + commit > + sample4.t > + commit > + sample3.t > + sample4.t > + EOF > + > + test_cmp expect actual > +' > + > test_done > -- > 2.30.1-633-gb004a83696 > >
