git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] diff: --{rotate,skip}-to=<path>
@ 2021-02-11 21:32 Junio C Hamano
  2021-02-12 16:15 ` Johannes Schindelin
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2021-02-11 21:32 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu

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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] diff: --{rotate,skip}-to=<path>
  2021-02-11 21:32 [PATCH] diff: --{rotate,skip}-to=<path> Junio C Hamano
@ 2021-02-12 16:15 ` Johannes Schindelin
  2021-02-12 17:54   ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Johannes Schindelin @ 2021-02-12 16:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, ZheNing Hu

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@pobox.com>
> ---
>
>  * 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
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] diff: --{rotate,skip}-to=<path>
  2021-02-12 16:15 ` Johannes Schindelin
@ 2021-02-12 17:54   ` Junio C Hamano
  0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2021-02-12 17:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, ZheNing Hu

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 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.

The use cases are quite different.  This one is meant for a
single-shot use, while orderfile is a more permanent thing.

See https://lore.kernel.org/git/xmqqeehp2jis.fsf@gitster.c.googlers.com/
and start reading from "I suspect that" for background story.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-02-12 17:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11 21:32 [PATCH] diff: --{rotate,skip}-to=<path> Junio C Hamano
2021-02-12 16:15 ` Johannes Schindelin
2021-02-12 17:54   ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).