[PATCH v3 0/1] Long names for `git log -S` and `git log -G`

[PATCH v3 0/1] Long names for `git log -S` and `git log -G`

[Illia Bobyr]

Same as PATCH v2[1], but removed gitk changes, as suggested by Johannes Sixt.

I'll send a separate patch for gitk, should this patch be accepted.
Or, I could include gitk changes into this chain, but just as a separate patch?

[1]: https://lore.kernel.org/git/20250205022422.2019929-1-illia.bobyr@gmail.com/

Illia Bobyr (1):
  diff: --patch-{modifies,grep} arg names for -S and -G

 Documentation/diff-options.txt |  36 +++++------
 Documentation/git-blame.txt    |   2 +-
 Documentation/gitdiffcore.txt  |  48 ++++++++-------
 diff.c                         |  18 +++---
 diff.h                         |  11 +++-
 t/t4062-diff-pickaxe.sh        |   8 +--
 t/t4209-log-pickaxe.sh         | 106 +++++++++++++++++++++++----------
 7 files changed, 142 insertions(+), 87 deletions(-)

-- 
2.45.2

[PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G

[Illia Bobyr]

Most arguments have both short and long versions.  Long versions are
easier to read, especially in scripts and command history.

Tests that check just the option parsing are duplicated to check both
short and long argument options.  But more complex tests are updated to
use the long argument in order to improve the test readability.
Assuming that the usage tests have already verified that both arguments
invoke the same underlying functionality.

Signed-off-by: Illia Bobyr <illia.bobyr@gmail.com>
---
 Documentation/diff-options.txt |  36 +++++------
 Documentation/git-blame.txt    |   2 +-
 Documentation/gitdiffcore.txt  |  48 ++++++++-------
 diff.c                         |  18 +++---
 diff.h                         |  11 +++-
 t/t4062-diff-pickaxe.sh        |   8 +--
 t/t4209-log-pickaxe.sh         | 106 +++++++++++++++++++++++----------
 7 files changed, 142 insertions(+), 87 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 640eb..c9f7c 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -650,6 +650,7 @@ Note that not all diffs can feature all types. For instance, copied and
 renamed entries cannot appear if detection for those types is disabled.
 
 `-S<string>`::
+`--patch-modifies=<string>`::
 	Look for differences that change the number of occurrences of
 	the specified _<string>_ (i.e. addition/deletion) in a file.
 	Intended for the scripter's use.
@@ -657,18 +658,19 @@ renamed entries cannot appear if detection for those types is disabled.
 It is useful when you're looking for an exact block of code (like a
 struct), and want to know the history of that block since it first
 came into being: use the feature iteratively to feed the interesting
-block in the preimage back into `-S`, and keep going until you get the
-very first version of the block.
+block in the preimage back into `--patch-modifies`, and keep going until
+you get the very first version of the block.
 +
 Binary files are searched as well.
 
 `-G<regex>`::
+`--patch-grep=<regex>`::
 	Look for differences whose patch text contains added/removed
 	lines that match _<regex>_.
 +
-To illustrate the difference between `-S<regex>` `--pickaxe-regex` and
-`-G<regex>`, consider a commit with the following diff in the same
-file:
+To illustrate the difference between `--patch-modifies=<regex>
+--pickaxe-regex` and `--patch-grep=<regex>`, consider a commit with the
+following diff in the same file:
 +
 ----
 +    return frotz(nitfol, two->ptr, 1, 0);
@@ -676,9 +678,9 @@ file:
 -    hit = frotz(nitfol, mf2.ptr, 1, 0);
 ----
 +
-While `git log -G"frotz\(nitfol"` will show this commit, `git log
--S"frotz\(nitfol" --pickaxe-regex` will not (because the number of
-occurrences of that string did not change).
+While `git log --patch-grep="frotz\(nitfol"` will show this commit, `git
+log --patch-modifies="frotz\(nitfol" --pickaxe-regex` will not (because the
+number of occurrences of that string did not change).
 +
 Unless `--text` is supplied patches of binary files without a textconv
 filter will be ignored.
@@ -687,22 +689,22 @@ See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more
 information.
 
 `--find-object=<object-id>`::
-	Look for differences that change the number of occurrences of
-	the specified object. Similar to `-S`, just the argument is different
-	in that it doesn't search for a specific string but for a specific
-	object id.
+	Look for differences that change the number of occurrences of the
+	specified object. Similar to `--patch-modifies`, just the argument
+	is different in that it doesn't search for a specific string but
+	for a specific object id.
 +
 The object can be a blob or a submodule commit. It implies the `-t` option in
 `git-log` to also find trees.
 
 `--pickaxe-all`::
-	When `-S` or `-G` finds a change, show all the changes in that
-	changeset, not just the files that contain the change
-	in _<string>_.
+	When `--patch-modifies` or `--patch-grep` finds a change, show all
+	the changes in that changeset, not just the files that contain the
+	change in _<string>_.
 
 `--pickaxe-regex`::
-	Treat the _<string>_ given to `-S` as an extended POSIX regular
-	expression to match.
+	Treat the _<string>_ given to `--patch-modifies` as an extended
+	POSIX regular expression to match.
 
 endif::git-format-patch[]
 
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index b1d7fb..0f21d3 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -41,7 +41,7 @@ a text string in the diff. A small example of the pickaxe interface
 that searches for `blame_usage`:
 
 -----------------------------------------------------------------------------
-$ git log --pretty=oneline -S'blame_usage'
+$ git log --pretty=oneline --patch-modifies='blame_usage'
 5040f17eba15504bad66b14a645bddd9b015ebb7 blame -S <ancestry-file>
 ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output
 -----------------------------------------------------------------------------
diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index 642c5..e4b18 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 
 This transformation limits the set of filepairs to those that change
 specified strings between the preimage and the postimage in a certain
-way.  -S<block-of-text> and -G<regular-expression> options are used to
-specify different ways these strings are sought.
-
-"-S<block-of-text>" detects filepairs whose preimage and postimage
-have different number of occurrences of the specified block of text.
-By definition, it will not detect in-file moves.  Also, when a
-changeset moves a file wholesale without affecting the interesting
-string, diffcore-rename kicks in as usual, and `-S` omits the filepair
-(since the number of occurrences of that string didn't change in that
+way.  --patch-modifies=<block-of-text> and
+--patch-grep=<regular-expression> options are used to specify
+different ways these strings are sought.
+
+"-S<block-of-text>", or "--patch-modifies=<block-of-text>" detects
+filepairs whose preimage and postimage have different number of
+occurrences of the specified block of text.  By definition, it will
+not detect in-file moves.  Also, when a changeset moves a file
+wholesale without affecting the interesting string, diffcore-rename
+kicks in as usual, and `--patch-modifies` omits the filepair (since
+the number of occurrences of that string didn't change in that
 rename-detected filepair).  When used with `--pickaxe-regex`, treat
 the <block-of-text> as an extended POSIX regular expression to match,
 instead of a literal string.
 
-"-G<regular-expression>" (mnemonic: grep) detects filepairs whose
-textual diff has an added or a deleted line that matches the given
-regular expression.  This means that it will detect in-file (or what
-rename-detection considers the same file) moves, which is noise.  The
-implementation runs diff twice and greps, and this can be quite
-expensive.  To speed things up, binary files without textconv filters
-will be ignored.
-
-When `-S` or `-G` are used without `--pickaxe-all`, only filepairs
-that match their respective criterion are kept in the output.  When
-`--pickaxe-all` is used, if even one filepair matches their respective
-criterion in a changeset, the entire changeset is kept.  This behavior
-is designed to make reviewing changes in the context of the whole
-changeset easier.
+"-G<regular-expression>", or "--patch-grep=<regular-expression>"
+(mnemonic: grep) detects filepairs whose textual diff has an added or
+a deleted line that matches the given regular expression.  This means
+that it will detect in-file (or what rename-detection considers the
+same file) moves, which is noise.  The implementation runs diff twice
+and greps, and this can be quite expensive.  To speed things up,
+binary files without textconv filters will be ignored.
+
+When `--patch-modifies` or `--patch-grep` are used without
+`--pickaxe-all`, only filepairs that match their respective criterion
+are kept in the output.  When `--pickaxe-all` is used, if even one
+filepair matches their respective criterion in a changeset, the entire
+changeset is kept.  This behavior is designed to make reviewing
+changes in the context of the whole changeset easier.
 
 diffcore-order: For Sorting the Output Based on Filenames
 ---------------------------------------------------------
diff --git a/diff.c b/diff.c
index d28b41..09beb 100644
--- a/diff.c
+++ b/diff.c
@@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
 		die(_("options '%s', '%s', and '%s' cannot be used together"),
-			"-G", "-S", "--find-object");
+			"-G/--patch-grep", "-S/--patch-modifies", "--find-object");
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))
 		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s'"),
-			"-G", "--pickaxe-regex", "--pickaxe-regex", "-S");
+			"-G/--patch-grep", "--pickaxe-regex",
+                        "--pickaxe-regex", "-S/--patch-modifies");
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))
 		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'"),
-			"--pickaxe-all", "--find-object", "--pickaxe-all", "-G", "-S");
+			"--pickaxe-all", "--find-object",
+                        "--pickaxe-all", "-G/--patch-grep", "-S/--patch-modifies");
 
 	/*
 	 * Most of the time we can say "there are changes"
@@ -5862,17 +5864,17 @@ struct option *add_diff_options(const struct option *opts,
 		OPT_SET_INT_F(0, "ita-visible-in-index", &options->ita_invisible_in_index,
 			      N_("treat 'git add -N' entries as real in the index"),
 			      0, PARSE_OPT_NONEG),
-		OPT_CALLBACK_F('S', NULL, options, N_("<string>"),
+		OPT_CALLBACK_F('S', "patch-modifies", options, N_("<string>"),
 			       N_("look for differences that change the number of occurrences of the specified string"),
 			       0, diff_opt_pickaxe_string),
-		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
-			       N_("look for differences that change the number of occurrences of the specified regex"),
+		OPT_CALLBACK_F('G', "patch-grep", options, N_("<regex>"),
+			       N_("look for differences where a patch contains the specified regex"),
 			       0, diff_opt_pickaxe_regex),
 		OPT_BIT_F(0, "pickaxe-all", &options->pickaxe_opts,
-			  N_("show all changes in the changeset with -S or -G"),
+			  N_("show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep"),
 			  DIFF_PICKAXE_ALL, PARSE_OPT_NONEG),
 		OPT_BIT_F(0, "pickaxe-regex", &options->pickaxe_opts,
-			  N_("treat <string> in -S as extended POSIX regular expression"),
+			  N_("treat <string> in -S/--patch-modifies as extended POSIX regular expression"),
 			  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),
 		OPT_FILENAME('O', NULL, &options->orderfile,
 			     N_("control the order in which files appear in the output")),
diff --git a/diff.h b/diff.h
index 6e6007..247ac 100644
--- a/diff.h
+++ b/diff.h
@@ -598,9 +598,16 @@ void diffcore_fix_diff_index(void);
 "                try unchanged files as candidate for copy detection.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
-"  -S<string>    find filepair whose only one side contains the string.\n" \
+"  -G<regex>\n" \
+"  --patch-grep=<regex>\n" \
+"                find differences whose patch contains the regex.\n" \
+"  -S<string>\n" \
+"  --patch-modifies=<string>\n" \
+"                find filepair who differ in the number of occurrences of string.\n" \
+"  --pickaxe-grep\n" \
+"                treat <string> as regex in the -S/--patch-modifies argument.\n" \
 "  --pickaxe-all\n" \
-"                show all files diff when -S is used and hit is found.\n" \
+"                show all files diff for -G/--patch-grep and -S/--patch-modifies.\n" \
 "  -a  --text    treat all files as text.\n"
 
 int diff_queue_is_empty(struct diff_options *o);
diff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh
index 8ad3d7..805e0f 100755
--- a/t/t4062-diff-pickaxe.sh
+++ b/t/t4062-diff-pickaxe.sh
@@ -16,13 +16,13 @@ test_expect_success setup '
 '
 
 # OpenBSD only supports up to 255 repetitions, so repeat twice for 64*64=4096.
-test_expect_success '-G matches' '
-	git diff --name-only -G "^(0{64}){64}$" HEAD^ >out &&
+test_expect_success '--patch-grep matches' '
+	git diff --name-only --patch-grep "^(0{64}){64}$" HEAD^ >out &&
 	test 4096-zeroes.txt = "$(cat out)"
 '
 
-test_expect_success '-S --pickaxe-regex' '
-	git diff --name-only -S0 --pickaxe-regex HEAD^ >out &&
+test_expect_success '--patch-modifies --pickaxe-regex' '
+	git diff --name-only --patch-modifies 0 --pickaxe-regex HEAD^ >out &&
 	test 4096-zeroes.txt = "$(cat out)"
 '
 
diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index a675ac..5f4d6 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='log --grep/--author/--regexp-ignore-case/-S/-G'
+test_description='log --grep/--author/--regexp-ignore-case/--patch-{modifies,grep}'
 
 . ./test-lib.sh
 
@@ -60,24 +60,48 @@ test_expect_success 'usage' '
 	test_expect_code 129 git log -S 2>err &&
 	test_grep "switch.*requires a value" err &&
 
+	test_expect_code 129 git log --patch-modifies 2>err &&
+	test_grep "option.*requires a value" err &&
+
 	test_expect_code 129 git log -G 2>err &&
 	test_grep "switch.*requires a value" err &&
 
+	test_expect_code 129 git log --patch-grep 2>err &&
+	test_grep "option.*requires a value" err &&
+
 	test_expect_code 128 git log -Gregex -Sstring 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log -Gregex --patch-modifies string 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex -Sstring 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex --patch-modifies string 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log --patch-grep regex --find-object=HEAD 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log --patch-modifies string --find-object=HEAD 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&
 	grep "cannot be used together" err
 '
 
 test_expect_success 'usage: --pickaxe-regex' '
 	test_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex --pickaxe-regex 2>err &&
 	grep "cannot be used together" err
 '
 
@@ -89,7 +113,13 @@ test_expect_success 'usage: --no-pickaxe-regex' '
 	test_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&
 	test_cmp expect actual &&
 
-	test_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&
+	test_expect_code 128 git log --patch-modifies string --no-pickaxe-regex 2>actual &&
+	test_cmp expect actual &&
+
+	test_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&
+	test_cmp expect actual &&
+
+	test_expect_code 128 git log --patch-grep regex --no-pickaxe-regex 2>err &&
 	test_cmp expect actual
 '
 
@@ -104,47 +134,59 @@ test_log_icase	expect_second	--author person
 test_log_icase	expect_nomatch	--author spreon
 
 test_log	expect_nomatch	-G picked
+test_log	expect_nomatch	--patch-grep picked
 test_log	expect_second	-G Picked
+test_log	expect_second	--patch-grep Picked
 test_log_icase	expect_nomatch	-G pickle
+test_log_icase	expect_nomatch	--patch-grep pickle
 test_log_icase	expect_second	-G picked
+test_log_icase	expect_second	--patch-grep picked
 
-test_expect_success 'log -G --textconv (missing textconv tool)' '
+test_expect_success 'log --patch-grep --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	test_must_fail git -c diff.test.textconv=missing log -Gfoo &&
+	test_must_fail git -c diff.test.textconv=missing log --patch-grep foo &&
 	rm .gitattributes
 '
 
-test_expect_success 'log -G --no-textconv (missing textconv tool)' '
+test_expect_success 'log --patch-grep --no-textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	git -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&
+	git -c diff.test.textconv=missing log --patch-grep foo --no-textconv >actual &&
 	test_cmp expect_nomatch actual &&
 	rm .gitattributes
 '
 
 test_log	expect_nomatch	-S picked
+test_log	expect_nomatch	--patch-modifies picked
 test_log	expect_second	-S Picked
+test_log	expect_second	--patch-modifies Picked
 test_log_icase	expect_second	-S picked
+test_log_icase	expect_second	--patch-modifies picked
 test_log_icase	expect_nomatch	-S pickle
+test_log_icase	expect_nomatch	--patch-modifies pickle
 
 test_log	expect_nomatch	-S p.cked --pickaxe-regex
+test_log	expect_nomatch	--patch-modifies p.cked --pickaxe-regex
 test_log	expect_second	-S P.cked --pickaxe-regex
+test_log	expect_second	--patch-modifies P.cked --pickaxe-regex
 test_log_icase	expect_second	-S p.cked --pickaxe-regex
+test_log_icase	expect_second	--patch-modifies p.cked --pickaxe-regex
 test_log_icase	expect_nomatch	-S p.ckle --pickaxe-regex
+test_log_icase	expect_nomatch	--patch-modifies p.ckle --pickaxe-regex
 
-test_expect_success 'log -S --textconv (missing textconv tool)' '
+test_expect_success 'log --patch-modifies --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	test_must_fail git -c diff.test.textconv=missing log -Sfoo &&
+	test_must_fail git -c diff.test.textconv=missing log --patch-modifies foo &&
 	rm .gitattributes
 '
 
-test_expect_success 'log -S --no-textconv (missing textconv tool)' '
+test_expect_success 'log --patch-modifies --no-textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	git -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&
+	git -c diff.test.textconv=missing log --patch-modifies foo --no-textconv >actual &&
 	test_cmp expect_nomatch actual &&
 	rm .gitattributes
 '
 
-test_expect_success 'setup log -[GS] plain & regex' '
+test_expect_success 'setup log --patch{-modifies,-grep} plain & regex' '
 	test_create_repo GS-plain &&
 	test_commit -C GS-plain --append A data.txt "a" &&
 	test_commit -C GS-plain --append B data.txt "a a" &&
@@ -159,31 +201,31 @@ test_expect_success 'setup log -[GS] plain & regex' '
 	git -C GS-plain log >full-log
 '
 
-test_expect_success 'log -G trims diff new/old [-+]' '
-	git -C GS-plain log -G"[+-]a" >log &&
+test_expect_success 'log --patch-grep trims diff new/old [-+]' '
+	git -C GS-plain log --patch-grep "[+-]a" >log &&
 	test_must_be_empty log &&
-	git -C GS-plain log -G"^a" >log &&
+	git -C GS-plain log --patch-grep "^a" >log &&
 	test_cmp log A-to-B-then-E-log
 '
 
-test_expect_success 'log -S<pat> is not a regex, but -S<pat> --pickaxe-regex is' '
-	git -C GS-plain log -S"a" >log &&
+test_expect_success 'log --patch-modifies <pat> is not a regex, but --patch-modifies <pat> --pickaxe-regex is' '
+	git -C GS-plain log --patch-modifies "a" >log &&
 	test_cmp log A-to-B-then-E-log &&
 
-	git -C GS-plain log -S"[a]" >log &&
+	git -C GS-plain log --patch-modifies "[a]" >log &&
 	test_must_be_empty log &&
 
-	git -C GS-plain log -S"[a]" --pickaxe-regex >log &&
+	git -C GS-plain log --patch-modifies "[a]" --pickaxe-regex >log &&
 	test_cmp log A-to-B-then-E-log &&
 
-	git -C GS-plain log -S"[b]" >log &&
+	git -C GS-plain log --patch-modifies "[b]" >log &&
 	test_cmp log D-then-E-log &&
 
-	git -C GS-plain log -S"[b]" --pickaxe-regex >log &&
+	git -C GS-plain log --patch-modifies "[b]" --pickaxe-regex >log &&
 	test_cmp log C-to-D-then-E-log
 '
 
-test_expect_success 'setup log -[GS] binary & --text' '
+test_expect_success 'setup log --patch{-modifies,-grep} binary & --text' '
 	test_create_repo GS-bin-txt &&
 	test_commit -C GS-bin-txt --printf A data.bin "a\na\0a\n" &&
 	test_commit -C GS-bin-txt --append --printf B data.bin "a\na\0a\n" &&
@@ -191,36 +233,36 @@ test_expect_success 'setup log -[GS] binary & --text' '
 	git -C GS-bin-txt log >full-log
 '
 
-test_expect_success 'log -G ignores binary files' '
-	git -C GS-bin-txt log -Ga >log &&
+test_expect_success 'log --patch-grep ignores binary files' '
+	git -C GS-bin-txt log --patch-grep a >log &&
 	test_must_be_empty log
 '
 
-test_expect_success 'log -G looks into binary files with -a' '
-	git -C GS-bin-txt log -a -Ga >log &&
+test_expect_success 'log --patch-grep looks into binary files with -a' '
+	git -C GS-bin-txt log -a --patch-grep a >log &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -G looks into binary files with textconv filter' '
+test_expect_success 'log --patch-grep looks into binary files with textconv filter' '
 	test_when_finished "rm GS-bin-txt/.gitattributes" &&
 	(
 		cd GS-bin-txt &&
 		echo "* diff=bin" >.gitattributes &&
-		git -c diff.bin.textconv=cat log -Ga >../log
+		git -c diff.bin.textconv=cat log --patch-grep a >../log
 	) &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -S looks into binary files' '
-	git -C GS-bin-txt log -Sa >log &&
+test_expect_success 'log --patch-modifies looks into binary files' '
+	git -C GS-bin-txt log --patch-modifies a >log &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -S --pickaxe-regex looks into binary files' '
-	git -C GS-bin-txt log --pickaxe-regex -Sa >log &&
+test_expect_success 'log --patch-modifies --pickaxe-regex looks into binary files' '
+	git -C GS-bin-txt log --pickaxe-regex --patch-modifies a >log &&
 	test_cmp log full-log &&
 
-	git -C GS-bin-txt log --pickaxe-regex -S"[a]" >log &&
+	git -C GS-bin-txt log --pickaxe-regex --patch-modifies "[a]" >log &&
 	test_cmp log full-log
 '
 
-- 
2.45.2

Re: [PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> Most arguments have both short and long versions.  Long versions are
> easier to read, especially in scripts and command history.
>
> Tests that check just the option parsing are duplicated to check both
> short and long argument options.  But more complex tests are updated to
> use the long argument in order to improve the test readability.

While checking both may be a prudent thing to do, because the "-S"
option and the "-G" option have been there with us almost since the
beginning of time, the swapping all existing use of them with the
longhand is rather unwelcome and needless churn, I would have to
say.

>  `-S<string>`::
> +`--patch-modifies=<string>`::

Good.  I am looking at 'git-commit.txt' as an example, and we seem
to give shorthand first and then longhand, which matches what we see
here.

> @@ -657,18 +658,19 @@ renamed entries cannot appear if detection for those types is disabled.
>  It is useful when you're looking for an exact block of code (like a
>  struct), and want to know the history of that block since it first
>  came into being: use the feature iteratively to feed the interesting
> -block in the preimage back into `-S`, and keep going until you get the
> -very first version of the block.
> +block in the preimage back into `--patch-modifies`, and keep going until
> +you get the very first version of the block.

If this paragraph _were_ written with the longhand from the
beginning, I would not have minded too much, but I personally find
it unnecessary to churn the existing document like this.

>  `-G<regex>`::
> +`--patch-grep=<regex>`::

Same two paragraphs from the above apply here, and ...

>  `--find-object=<object-id>`::
>  `--pickaxe-all`::
>  `--pickaxe-regex`::

... all of the above.

> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt

Ditto.

> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
> index 642c5..e4b18 100644
> --- a/Documentation/gitdiffcore.txt
> +++ b/Documentation/gitdiffcore.txt
> @@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
>  
>  This transformation limits the set of filepairs to those that change
>  specified strings between the preimage and the postimage in a certain
> -way.  -S<block-of-text> and -G<regular-expression> options are used to
> +way.  --patch-modifies=<block-of-text> and
> +--patch-grep=<regular-expression> options are used to specify
> +different ways these strings are sought.

This is worse.  Here is the first part that describes the pickaxe,
so mentioning both may be more appropriate; showing only the
longhand nobody is familiar with (yet) does not make any sense.

    ... certain way.  `--patch-modifies=<block-of-text>`
    (`-S<block-of-text>` for short) and `--patch-grep=<regular-expression>`
    (`-G<regular-expression>` for short) are used to ...

Once establishing the equivalence between the longhand and the
shorthand for these two options, we do not have to churn the
existing text at all.

> diff --git a/diff.c b/diff.c
> index d28b41..09beb 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)
>  
>  	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
>  		die(_("options '%s', '%s', and '%s' cannot be used together"),
> -			"-G", "-S", "--find-object");
> +			"-G/--patch-grep", "-S/--patch-modifies", "--find-object");
>  
>  	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))
>  		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s'"),
> -			"-G", "--pickaxe-regex", "--pickaxe-regex", "-S");
> +			"-G/--patch-grep", "--pickaxe-regex",
> +                        "--pickaxe-regex", "-S/--patch-modifies");
>  
>  	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))
>  		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'"),
> -			"--pickaxe-all", "--find-object", "--pickaxe-all", "-G", "-S");
> +			"--pickaxe-all", "--find-object",
> +                        "--pickaxe-all", "-G/--patch-grep", "-S/--patch-modifies");

The message change looks fine; the indentation is broken.

.git/rebase-apply/patch:184: indent with spaces.
                        "--pickaxe-regex", "-S/--patch-modifies");
.git/rebase-apply/patch:190: indent with spaces.
                        "--pickaxe-all", "-G/--patch-grep", "-S/--patch-modifies");
warning: 2 lines applied after fixing whitespace errors.
Applying: diff: --patch-{modifies,grep} arg names for -S and -G

These alone do not require a new iteration, as "git am --whitespace=fix"
already corrected them.

> -		OPT_CALLBACK_F('S', NULL, options, N_("<string>"),
> +		OPT_CALLBACK_F('S', "patch-modifies", options, N_("<string>"),
> -		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
> +		OPT_CALLBACK_F('G', "patch-grep", options, N_("<regex>"),

OK.  NOte that this says <regex>.  We may want to have a separate clean-up
patch so that Documentation/gitdifcore.txt that used <regular-expression>
and the placeholder used here match.

> -			       N_("look for differences that change the number of occurrences of the specified regex"),
> +			       N_("look for differences where a patch contains the specified regex"),

This is an unrelated change that should not be in this patch.  If
you want to modify it, please do it in a separate clean-up patch,
just like the above <regex> vs <regular-expression> change.

> -			  N_("show all changes in the changeset with -S or -G"),
> +			  N_("show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep"),

This line is meant to be shown when the user requests list of
options and their meanings.  Growing the message from 47 columns or
so to 78 columns would make it wider than terminals when these
messages are indented.  Because earlier entries in this array have
already established the equivalence between the shorthand and the
longhand, I do not think the output is understandable without this
change.

>  		OPT_BIT_F(0, "pickaxe-regex", &options->pickaxe_opts,
> -			  N_("treat <string> in -S as extended POSIX regular expression"),
> +			  N_("treat <string> in -S/--patch-modifies as extended POSIX regular expression"),

Ditto.

Thanks.

Re: [PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G

[Illia Bobyr]

On 2/6/25 12:59, Junio C Hamano wrote:
 > Illia Bobyr <illia.bobyr@gmail.com> writes:
 >
 >> Most arguments have both short and long versions.  Long versions are
 >> easier to read, especially in scripts and command history.
 >>
 >> Tests that check just the option parsing are duplicated to check both
 >> short and long argument options.  But more complex tests are updated to
 >> use the long argument in order to improve the test readability.
 >
 > While checking both may be a prudent thing to do, because the "-S"
 > option and the "-G" option have been there with us almost since the
 > beginning of time, the swapping all existing use of them with the
 > longhand is rather unwelcome and needless churn, I would have to
 > say.

My thinking is that as long version names improve readability, it also 
applies
to the test code.  When I see a short option, I often have to check the 
manual
to remember what exactly does it do.  Even for "-S"/"-G" I find myself 
sometimes
confused as to which of the two does what exactly.  While the "grep" 
mnemonic
helps, I do not always remember it.

But, I think, I understand your point of view as well.

In v5, patch 5 contains a relatively minimum amount of changes that add long
alternatives for "-S" and "-G" just to the command line parsing.

I do not have your experience with assessing the churn, but if my 
argument about
the readability changes your mind, I've moved the rest of the updates into
separate patches, at the end of the chain.  Patches 8 through 10. Making it
easier to discuss them in smaller chunks, if you wish so.  But also, I 
assume,
it should be easy for you to ignore those, if you do not want to include 
them?

 > [...]
 >
 >> diff --git a/Documentation/gitdiffcore.txt 
b/Documentation/gitdiffcore.txt
 >> index 642c5..e4b18 100644
 >> --- a/Documentation/gitdiffcore.txt
 >> +++ b/Documentation/gitdiffcore.txt
 >> @@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting 
Addition/Deletion of Specified String
 >>
 >>  This transformation limits the set of filepairs to those that change
 >>  specified strings between the preimage and the postimage in a certain
 >> -way.  -S<block-of-text> and -G<regular-expression> options are used to
 >> +way.  --patch-modifies=<block-of-text> and
 >> +--patch-grep=<regular-expression> options are used to specify
 >> +different ways these strings are sought.
 >
 > This is worse.  Here is the first part that describes the pickaxe,
 > so mentioning both may be more appropriate; showing only the
 > longhand nobody is familiar with (yet) does not make any sense.
 >
 >     ... certain way. `--patch-modifies=<block-of-text>`
 >     (`-S<block-of-text>` for short) and 
`--patch-grep=<regular-expression>`
 >     (`-G<regular-expression>` for short) are used to ...
 >
 > Once establishing the equivalence between the longhand and the
 > shorthand for these two options, we do not have to churn the
 > existing text at all.

Applied your suggestion.

I guess one difference in the way you look at it, is that you default to the
short version when you can.  While I default to the long one, as I 
assume it is
easier to understand.  Someone not that intimately familiar with git 
might need
to go to the previous paragraph to recall what "-S" and "-G" are, while 
if they
are spelled as "--patch-modifies" and "--patch-grep", it might be less
necessary.  So, the argument is that while we are reducing the diff, we 
might
also be reducing the improvement in readability.

Being the author, I could also be biased when assessing how much more 
readable
"--patch-modifies" and "--patch-grep" are compared to "-S" and "-G".

 >> diff --git a/diff.c b/diff.c
 >> index d28b41..09beb 100644
 >> --- a/diff.c
 >> +++ b/diff.c
 >> @@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options 
*options)
 >>
 >>      if (HAS_MULTI_BITS(options->pickaxe_opts & 
DIFF_PICKAXE_KINDS_MASK))
 >>          die(_("options '%s', '%s', and '%s' cannot be used together"),
 >> -            "-G", "-S", "--find-object");
 >> +            "-G/--patch-grep", "-S/--patch-modifies", "--find-object");
 >>
 >>      if (HAS_MULTI_BITS(options->pickaxe_opts & 
DIFF_PICKAXE_KINDS_G_REGEX_MASK))
 >>          die(_("options '%s' and '%s' cannot be used together, use 
'%s' with '%s'"),
 >> -            "-G", "--pickaxe-regex", "--pickaxe-regex", "-S");
 >> +            "-G/--patch-grep", "--pickaxe-regex",
 >> +                        "--pickaxe-regex", "-S/--patch-modifies");
 >>
 >>      if (HAS_MULTI_BITS(options->pickaxe_opts & 
DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))
 >>          die(_("options '%s' and '%s' cannot be used together, use 
'%s' with '%s' and '%s'"),
 >> -            "--pickaxe-all", "--find-object", "--pickaxe-all", 
"-G", "-S");
 >> +            "--pickaxe-all", "--find-object",
 >> +                        "--pickaxe-all", "-G/--patch-grep", 
"-S/--patch-modifies");
 >
 > The message change looks fine; the indentation is broken.
 >
 > .git/rebase-apply/patch:184: indent with spaces.
 >                         "--pickaxe-regex", "-S/--patch-modifies");
 > .git/rebase-apply/patch:190: indent with spaces.
 >                         "--pickaxe-all", "-G/--patch-grep", 
"-S/--patch-modifies");
 > warning: 2 lines applied after fixing whitespace errors.
 > Applying: diff: --patch-{modifies,grep} arg names for -S and -G
 >
 > These alone do not require a new iteration, as "git am --whitespace=fix"
 > already corrected them.

Sorry about this.  I did check the indentation manually, but did not use a
tool.  Reconfigured my editor to use tabs now.

 >> -        OPT_CALLBACK_F('S', NULL, options, N_("<string>"),
 >> +        OPT_CALLBACK_F('S', "patch-modifies", options, N_("<string>"),
 >> -        OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
 >> +        OPT_CALLBACK_F('G', "patch-grep", options, N_("<regex>"),
 >
 > OK.  NOte that this says <regex>.  We may want to have a separate 
clean-up
 > patch so that Documentation/gitdifcore.txt that used <regular-expression>
 > and the placeholder used here match.

Makes sense.
I've added this fix as patch 5 in v5.
I've reformatted paragraphs in gitdifcore.adoc that were affected.
Let me know if you do not want me to reformat it, and just keep shorter 
lines as
is.

 >> -                   N_("look for differences that change the number 
of occurrences of the specified regex"),
 >> +                   N_("look for differences where a patch contains 
the specified regex"),
 >
 > This is an unrelated change that should not be in this patch.  If
 > you want to modify it, please do it in a separate clean-up patch,
 > just like the above <regex> vs <regular-expression> change.

Split it into patch 2 in v5.

 >> -              N_("show all changes in the changeset with -S or -G"),
 >> +              N_("show all changes in the changeset with 
-S/--patch-modifies or -G/--patch-grep"),
 >
 > This line is meant to be shown when the user requests list of
 > options and their meanings.  Growing the message from 47 columns or
 > so to 78 columns would make it wider than terminals when these
 > messages are indented.  Because earlier entries in this array have
 > already established the equivalence between the shorthand and the
 > longhand, I do not think the output is understandable without this
 > change.

A description for "-S" is already 81 characters long:

N_("look for differences that change the number of occurrences of the 
specified regex"),

So I was assuming if I grow another description to 77 characters, it is 
still OK.
While one can find the correspondence between "-S" and "--patch-modifies" by
reading the "-S" description, in my mind, the same argument applies as 
to the
test readability - it just makes it a bit easier for the reader.

This change is now part of a much smaller patch 9 in v5, which is only about
adding longer alternatives to the CLI help messages that currently 
contain only
"-G" and "-S".  This way you can decide if you want it or not as a complete
unit.  Or if you want me to change it in some way, we can discuss it 
separately
from the rest of the changes.

By the way, I must admit I can not find a way to look at a help message
generated from these strings.  Running `git diff -h` shows a message from

`diff.h` and running `git diff --help` shows the man page.

Thank you.

Re: [PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> My thinking is that as long version names improve readability, it also
> applies
> to the test code.  When I see a short option, I often have to check
> the manual
> to remember what exactly does it do.

But by now due to enough exposure, you have committed them in your
memory, no? ;-)

> But, I think, I understand your point of view as well.

Yup, if the options were introduced with long and short forms at the
same time and the tests were written at the same time or shortly
after their introduction, I'd agree that using longer form more may
be beneficial, since there is nobody who is already familier with
either of the forms.  But at this point after 20 years, swapping one
for the other is mostly unnecessary churn, I would have to say (and
I do not particularly want to having to repeat saying the same thing
again).

>> OK.  NOte that this says <regex>.  We may want to have a separate
>   clean-up
>> patch so that Documentation/gitdifcore.txt that used <regular-expression>
>> and the placeholder used here match.
>
> Makes sense.
> I've added this fix as patch 5 in v5.

I'd rather see these "fixes to existing anomalies" done totally
outside of this series.  IOW, I'd prefer to either (1) get the
series done with the minimally necessary changes first and then
after the dust settles from merging that to 'master', see these "oh
we noticed these issues while working on the other series that has
now completed" issues addressed, or (2) do the clean-up of existing
anomalies first as a separate series, and then after the dust
settles for the clean-up, do the proposed addition of longform as a
separate series.  I have slight preference to (1), simply because
nobody complained on these small anomalies for the past 20 years ;-)
but I can also go with "preliminary clean-up first" route.

>> This is an unrelated change that should not be in this patch.  If
>> you want to modify it, please do it in a separate clean-up patch,
>> just like the above <regex> vs <regular-expression> change.
>
> Split it into patch 2 in v5.

Again, when I said "unrelated", I meant that I want them to be
treated as unrelated changes, addressed outside of this series,
either in a preliminary clean-up, or after-the-dust-settles
clean-up.

Thanks.

Re: [PATCH v3 0/1] Long names for `git log -S` and `git log -G`

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> Same as PATCH v2[1], but removed gitk changes, as suggested by Johannes Sixt.
>
> I'll send a separate patch for gitk, should this patch be accepted.
> Or, I could include gitk changes into this chain, but just as a separate patch?

They are technically separate codebases so the changes to the core
would become prerequisite for the same changes to gitk, so from that
point of view the former may be more kosher, but I would expect that
the latter is fine in practice.

[PATCH v4 0/10] Long names for `git log -S` and `git log -G`

[Illia Bobyr]

I've split the big change from v3 [1] into multiple, mostly independent patches
to make it easier to review and merge each one separately.

[1] https://lore.kernel.org/git/20250206014324.1839232-1-illia.bobyr@gmail.com/

Patches 1 through 4 are fixing minor bugs and inconsistencies.

Patch 5 contains updates gitdiffcore to use same placeholder names as the rest
of the code.

Patch 6 contains a minimum change to add long versions of -S and -G.

Patch 7 adds bash completion support.

Patches 8 through 10 increase usage of the long argument versions in tests, CLI
help and docs respectively.

Please, let me know if you prefer it split in a different way, or reorder the
changes.

---

I was not sure if I should include a reference to the previous version of the
patch into the next reroll.  It seems that
`Documentation/MyFirstContribution.adoc` suggests so.  But it creates very long
threads.  And I've noticed that not everyone is doing it.

---

Reply to review notes from Junio C Hamano:

On 2/6/25 12:59, Junio C Hamano wrote:
> Illia Bobyr <illia.bobyr@gmail.com> writes:
>
>> Most arguments have both short and long versions.  Long versions are
>> easier to read, especially in scripts and command history.
>>
>> Tests that check just the option parsing are duplicated to check both
>> short and long argument options.  But more complex tests are updated to
>> use the long argument in order to improve the test readability.
>
> While checking both may be a prudent thing to do, because the "-S"
> option and the "-G" option have been there with us almost since the
> beginning of time, the swapping all existing use of them with the
> longhand is rather unwelcome and needless churn, I would have to
> say.

My thinking is that as long version names improve readability, it also applies
to the test code.  When I see a short option I often have to check the manual to
remember what exactly does it do.  Even for "-S"/"-G" I find myself sometimes
confused as to which of the two does what exactly.  While the "grep" mnemonic
helps, I do not always remember it.

But, I think, I understand your point of view as well.

In v4 patch 5 contains a relatively minimum amount of changes that add long
alternatives for "-S" and "-G" just to the command line parsing.

I do not have your experience with assessing the churn, but if my argument about
the readability changes your mind, I've moved the rest of the updates into
separate patches, at the end of the chain.  Patches 8 through 10.  Making it
easier to discuss them in smaller chunks, if you wish so.  But also, I assume,
it should be easy for you to ignore those, if you do not want to include them?

> [...]
>> @@ -657,18 +658,19 @@ renamed entries cannot appear if detection for those types is disabled.
>>  It is useful when you're looking for an exact block of code (like a
>>  struct), and want to know the history of that block since it first
>>  came into being: use the feature iteratively to feed the interesting
>> -block in the preimage back into `-S`, and keep going until you get the
>> -very first version of the block.
>> +block in the preimage back into `--patch-modifies`, and keep going until
>> +you get the very first version of the block.
>
> If this paragraph _were_ written with the longhand from the
> beginning, I would not have minded too much, but I personally find
> it unnecessary to churn the existing document like this.
>
>>  `-G<regex>`::
>> +`--patch-grep=<regex>`::
>
> Same two paragraphs from the above apply here, and ...
>
>>  `--find-object=<object-id>`::
>>  `--pickaxe-all`::
>>  `--pickaxe-regex`::
>
> ... all of the above.
>
>> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
>
> Ditto.
>
>> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
>> index 642c5..e4b18 100644
>> --- a/Documentation/gitdiffcore.txt
>> +++ b/Documentation/gitdiffcore.txt
>> @@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
>>  
>>  This transformation limits the set of filepairs to those that change
>>  specified strings between the preimage and the postimage in a certain
>> -way.  -S<block-of-text> and -G<regular-expression> options are used to
>> +way.  --patch-modifies=<block-of-text> and
>> +--patch-grep=<regular-expression> options are used to specify
>> +different ways these strings are sought.
>
> This is worse.  Here is the first part that describes the pickaxe,
> so mentioning both may be more appropriate; showing only the
> longhand nobody is familiar with (yet) does not make any sense.
>
>     ... certain way.  `--patch-modifies=<block-of-text>`
>     (`-S<block-of-text>` for short) and `--patch-grep=<regular-expression>`
>     (`-G<regular-expression>` for short) are used to ...
>
> Once establishing the equivalence between the longhand and the
> shorthand for these two options, we do not have to churn the
> existing text at all.

Applied your suggestion.

I guess one difference in the way you look at it, is that you default to the
short version when you can.  While I default to the long one, as I assume it is
easier to understand.  Someone not that intimately familiar with git might need
to go to the previous paragraph to recall what "-S" and "-G" are, while if they
are spelled as "--patch-modifies" and "--patch-grep", it might be less
necessary.  So, the argument is that while we are reducing the diff, we might
also be reducing the improvement in readability.

Being the author, I could also be biased when assessing how much more readable
"--patch-modifies" and "--patch-grep" are compared to "-S" and "-G".

>> diff --git a/diff.c b/diff.c
>> index d28b41..09beb 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)
>>  
>>  	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
>>  		die(_("options '%s', '%s', and '%s' cannot be used together"),
>> -			"-G", "-S", "--find-object");
>> +			"-G/--patch-grep", "-S/--patch-modifies", "--find-object");
>>  
>>  	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))
>>  		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s'"),
>> -			"-G", "--pickaxe-regex", "--pickaxe-regex", "-S");
>> +			"-G/--patch-grep", "--pickaxe-regex",
>> +                        "--pickaxe-regex", "-S/--patch-modifies");
>>  
>>  	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))
>>  		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'"),
>> -			"--pickaxe-all", "--find-object", "--pickaxe-all", "-G", "-S");
>> +			"--pickaxe-all", "--find-object",
>> +                        "--pickaxe-all", "-G/--patch-grep", "-S/--patch-modifies");
>
> The message change looks fine; the indentation is broken.
>
> .git/rebase-apply/patch:184: indent with spaces.
>                         "--pickaxe-regex", "-S/--patch-modifies");
> .git/rebase-apply/patch:190: indent with spaces.
>                         "--pickaxe-all", "-G/--patch-grep", "-S/--patch-modifies");
> warning: 2 lines applied after fixing whitespace errors.
> Applying: diff: --patch-{modifies,grep} arg names for -S and -G
>
> These alone do not require a new iteration, as "git am --whitespace=fix"
> already corrected them.

Sorry about this.  I did check the indentation manually, but did not use a
tool.  Reconfigured my editor to use tabs now.

>> -		OPT_CALLBACK_F('S', NULL, options, N_("<string>"),
>> +		OPT_CALLBACK_F('S', "patch-modifies", options, N_("<string>"),
>> -		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
>> +		OPT_CALLBACK_F('G', "patch-grep", options, N_("<regex>"),
>
> OK.  NOte that this says <regex>.  We may want to have a separate clean-up
> patch so that Documentation/gitdifcore.txt that used <regular-expression>
> and the placeholder used here match.

Makes sense.
I've added this fix as patch 5.
I've reformatted paragraphs in gitdifcore.adoc that were affected.
Let me know if you do not want me to reformat it, and just keep shorter lines as
is.

>> -			       N_("look for differences that change the number of occurrences of the specified regex"),
>> +			       N_("look for differences where a patch contains the specified regex"),
>
> This is an unrelated change that should not be in this patch.  If
> you want to modify it, please do it in a separate clean-up patch,
> just like the above <regex> vs <regular-expression> change.

Split it into patch 2.

>> -			  N_("show all changes in the changeset with -S or -G"),
>> +			  N_("show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep"),
>
> This line is meant to be shown when the user requests list of
> options and their meanings.  Growing the message from 47 columns or
> so to 78 columns would make it wider than terminals when these
> messages are indented.  Because earlier entries in this array have
> already established the equivalence between the shorthand and the
> longhand, I do not think the output is understandable without this
> change.

A description for "-S" is already 81 characters long:

N_("look for differences that change the number of occurrences of the specified regex"),

So I was assuming if I grow another description to 77 characters it is still OK.
While one can find the correspondence between "-S" and "--patch-modifies" by
reading the "-S" description, in my mind, the same argument applies as to the
test readability - it just makes it a bit easier for the reader.

This change is now part of a much smaller patch 9, which is only about adding
longer alternatives to the CLI help messages that currently contain only "-G"
and "-S".  This way you can decide if you want it or not as a complete unit.  Or
if you want me to change it in some way, we can discuss it separately from the
rest of the changes.

By the way, I must admit I can not find a way to look at a help message
generated from these strings.  Running `git diff -h` shows a message from
`diff.h` and running `git diff --help` shows the man page.

---

Illia Bobyr (10):
  t/t4209-log-pickaxe: Naming typo: -G takes a regex
  diff: -G description: Correct copy/paste error
  diff: short help: Correct -S description
  diff: short help: Add -G and --pickaxe-grep
  docs: gitdiffcore: -G and -S: Use regex/string placeholders
  diff: --patch-{grep,modifies} arg names for -G and -S
  completion: Support --patch-{grep,modifies}
  diff: test: Use --patch-{grep,modifies} over -G/-S
  diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}
  diff: docs: Use --patch-{grep,modifies} over -G/-S

 Documentation/diff-options.adoc        |  36 +++++----
 Documentation/git-blame.adoc           |   2 +-
 Documentation/gitdiffcore.adoc         |  55 ++++++-------
 contrib/completion/git-completion.bash |  11 ++-
 diff.c                                 |  18 +++--
 diff.h                                 |  11 ++-
 t/t4062-diff-pickaxe.sh                |   8 +-
 t/t4209-log-pickaxe.sh                 | 106 +++++++++++++++++--------
 8 files changed, 155 insertions(+), 92 deletions(-)

-- 
2.45.2

Re: [PATCH v4 0/10] Long names for `git log -S` and `git log -G`

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> I've split the big change from v3 [1] into multiple, mostly independent patches
> to make it easier to review and merge each one separately.
>
> [1] https://lore.kernel.org/git/20250206014324.1839232-1-illia.bobyr@gmail.com/
>
> Patches 1 through 4 are fixing minor bugs and inconsistencies.
>
> Patch 5 contains updates gitdiffcore to use same placeholder names as the rest
> of the code.
>
> Patch 6 contains a minimum change to add long versions of -S and -G.
>
> Patch 7 adds bash completion support.
>
> Patches 8 through 10 increase usage of the long argument versions in tests, CLI
> help and docs respectively.
>
> Please, let me know if you prefer it split in a different way, or reorder the
> changes.

When you base your patch on a different base than 'master' (or if
the previous iteration of the topic has already been queued in my
tree, then the commit used as the base to queue the topic), please
make sure you state it clearly.  

This iteration seems to apply on none of bc204b74 (The seventh
batch, 2025-02-03), on top of which the previous round dcc02caba2
(ib/diff-S-G-with-longhand) has been queued, or any of the recent
tips of 'master', like 388218fa (The ninth batch, 2025-02-10) or
9520f7d9 (The eighth batch, 2025-02-06), so I cannot look at it.

> I was not sure if I should include a reference to the previous version of the
> patch into the next reroll.  It seems that
> `Documentation/MyFirstContribution.adoc` suggests so.  But it creates very long
> threads.  And I've noticed that not everyone is doing it.

Almost everybody does so, actually.

Taking a topic that has 5 iterations, each about ~20 patches, as an
example:

https://lore.kernel.org/git/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im/

it is perfectly clear and easy to nagivate from the list of messages
what discussions we had in previous iterations.

> Reply to review notes ...

It is more customary to Reply-all directly to review messages,
instead of sending new round of patches.  When the cover letter of a
new iteration is sent as a response to the cover letter of the
previous iteration, readers can find the previous discussion
messages.

Thanks.

Re: [PATCH v4 0/10] Long names for `git log -S` and `git log -G`

[Illia Bobyr]

On 2/11/25 10:07, Junio C Hamano wrote:
 > Illia Bobyr <illia.bobyr@gmail.com> writes:
 >
 >> I've split the big change from v3 [1] into multiple, mostly 
independent patches
 >> to make it easier to review and merge each one separately.
 >>
 >> [1] 
https://lore.kernel.org/git/20250206014324.1839232-1-illia.bobyr@gmail.com/
 >>
 >> Patches 1 through 4 are fixing minor bugs and inconsistencies.
 >>
 >> Patch 5 contains updates gitdiffcore to use same placeholder names 
as the rest
 >> of the code.
 >>
 >> Patch 6 contains a minimum change to add long versions of -S and -G.
 >>
 >> Patch 7 adds bash completion support.
 >>
 >> Patches 8 through 10 increase usage of the long argument versions in 
tests, CLI
 >> help and docs respectively.
 >>
 >> Please, let me know if you prefer it split in a different way, or 
reorder the
 >> changes.
 >
 > When you base your patch on a different base than 'master' (or if
 > the previous iteration of the topic has already been queued in my
 > tree, then the commit used as the base to queue the topic), please
 > make sure you state it clearly.
 >
 > This iteration seems to apply on none of bc204b74 (The seventh
 > batch, 2025-02-03), on top of which the previous round dcc02caba2
 > (ib/diff-S-G-with-longhand) has been queued, or any of the recent
 > tips of 'master', like 388218fa (The ninth batch, 2025-02-10) or
 > 9520f7d9 (The eighth batch, 2025-02-06), so I cannot look at it.

Sorry for the confusion.  I randomly decided to check if my changes have any
conflicts with `next` and rebased on top of it.
Did not realize it would affect the patches.
I've rebased back on top of `master` and published as v5.

 >> I was not sure if I should include a reference to the previous 
version of the
 >> patch into the next reroll.  It seems that
 >> `Documentation/MyFirstContribution.adoc` suggests so. But it creates 
very long
 >> threads.  And I've noticed that not everyone is doing it.
 >
 > Almost everybody does so, actually.
 >
 > Taking a topic that has 5 iterations, each about ~20 patches, as an
 > example:
 >
 > 
https://lore.kernel.org/git/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im/
 >
 > it is perfectly clear and easy to nagivate from the list of messages
 > what discussions we had in previous iterations.

Thank you for the explanation and for sharing an example link. I'll use v3
cover letter as a reference point for v5, as I have already interrupted the
reference chain in v3.

 >> Reply to review notes ...
 >
 > It is more customary to Reply-all directly to review messages,
 > instead of sending new round of patches.  When the cover letter of a
 > new iteration is sent as a response to the cover letter of the
 > previous iteration, readers can find the previous discussion
 > messages.

Got it.  Thank you.  I have replied to your review email, so that we can
continue the conversation there.

[PATCH v4 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex

[Illia Bobyr]

Not effect on the test logic, but as "-G" argument is a regex it is more
accurate to use "regex" as a dummy argument value rather than "string".
In all the other case when "-G" is passed a dummy value it is spelled as
"regex" rather than as "string".
---
 t/t4209-log-pickaxe.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index a675ac..ed70c 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -89,7 +89,7 @@ test_expect_success 'usage: --no-pickaxe-regex' '
 	test_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&
 	test_cmp expect actual &&
 
-	test_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&
+	test_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&
 	test_cmp expect actual
 '
 
-- 
2.45.2

[PATCH v4 02/10] diff: -G description: Correct copy/paste error

[Illia Bobyr]

Current description for -G is incorrect, seems like it was copied from
the description for -S.
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 019fb..bd9db 100644
--- a/diff.c
+++ b/diff.c
@@ -5866,7 +5866,7 @@ struct option *add_diff_options(const struct option *opts,
 			       N_("look for differences that change the number of occurrences of the specified string"),
 			       0, diff_opt_pickaxe_string),
 		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
-			       N_("look for differences that change the number of occurrences of the specified regex"),
+			       N_("look for differences where a patch contains the specified regex"),
 			       0, diff_opt_pickaxe_regex),
 		OPT_BIT_F(0, "pickaxe-all", &options->pickaxe_opts,
 			  N_("show all changes in the changeset with -S or -G"),
-- 
2.45.2

[PATCH v4 03/10] diff: short help: Correct -S description

[Illia Bobyr]

`-S` shows changes that modify the number of occurrences of the
specified string, rather than only those that either completely remove
it or add it for the first time.
---
 diff.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.h b/diff.h
index 0a566f..49ece3 100644
--- a/diff.h
+++ b/diff.h
@@ -606,7 +606,7 @@ void diffcore_fix_diff_index(void);
 "                try unchanged files as candidate for copy detection.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
-"  -S<string>    find filepair whose only one side contains the string.\n" \
+"  -S<string>    find filepair who differ in the number of occurrences of string.\n" \
 "  --pickaxe-all\n" \
 "                show all files diff when -S is used and hit is found.\n" \
 "  -a  --text    treat all files as text.\n"
-- 
2.45.2

[PATCH v4 04/10] diff: short help: Add -G and --pickaxe-grep

[Illia Bobyr]

-G and --pickaxe-grep seems to be on par with -S and --pickaxe-all that
are already mentioned.
---
 diff.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/diff.h b/diff.h
index 49ece3..787bb 100644
--- a/diff.h
+++ b/diff.h
@@ -606,9 +606,12 @@ void diffcore_fix_diff_index(void);
 "                try unchanged files as candidate for copy detection.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
+"  -G<regex>     find differences where patch contains the specified regex.\n" \
 "  -S<string>    find filepair who differ in the number of occurrences of string.\n" \
+"  --pickaxe-grep\n" \
+"                treat <string> as a regex in the -S argument.\n" \
 "  --pickaxe-all\n" \
-"                show all files diff when -S is used and hit is found.\n" \
+"                show all files diff when -G or -S is used and hit is found.\n" \
 "  -a  --text    treat all files as text.\n"
 
 int diff_queue_is_empty(struct diff_options *o);
-- 
2.45.2

[PATCH v4 05/10] docs: gitdiffcore: -G and -S: Use regex/string placeholders

[Illia Bobyr]

In the rest of the documentation (and in the code) we use `regex` and
`string` as `-G` and `-S` argument placeholders.  While
`regular-expression` and `block-of-text` are a bit easier to read, it is
a bit consistent.

And we could assume that everyone who uses git should be able to
understand that a "string" and a "block-of-text", as well as a "regex"
and "regular-expression" are the same thing.  So, using a shorter
version is also more consistent.
---
 Documentation/gitdiffcore.adoc | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/Documentation/gitdiffcore.adoc b/Documentation/gitdiffcore.adoc
index 642c5..0d7d66 100644
--- a/Documentation/gitdiffcore.adoc
+++ b/Documentation/gitdiffcore.adoc
@@ -245,26 +245,25 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 
 This transformation limits the set of filepairs to those that change
 specified strings between the preimage and the postimage in a certain
-way.  -S<block-of-text> and -G<regular-expression> options are used to
-specify different ways these strings are sought.
+way.  `-S<string>` and `-G<regex>` options are used to specify
+different ways these strings are sought.
 
-"-S<block-of-text>" detects filepairs whose preimage and postimage
-have different number of occurrences of the specified block of text.
+`-S<string>` detects filepairs whose preimage and postimage
+have different number of occurrences of the specified _<string>_.
 By definition, it will not detect in-file moves.  Also, when a
 changeset moves a file wholesale without affecting the interesting
 string, diffcore-rename kicks in as usual, and `-S` omits the filepair
 (since the number of occurrences of that string didn't change in that
 rename-detected filepair).  When used with `--pickaxe-regex`, treat
-the <block-of-text> as an extended POSIX regular expression to match,
+the _<string>_ as an extended POSIX regular expression to match,
 instead of a literal string.
 
-"-G<regular-expression>" (mnemonic: grep) detects filepairs whose
-textual diff has an added or a deleted line that matches the given
-regular expression.  This means that it will detect in-file (or what
-rename-detection considers the same file) moves, which is noise.  The
-implementation runs diff twice and greps, and this can be quite
-expensive.  To speed things up, binary files without textconv filters
-will be ignored.
+`-G<regex>` (mnemonic: grep) detects filepairs whose textual diff has
+an added or a deleted line that matches the given _<regex>_.  This
+means that it will detect in-file (or what rename-detection considers
+the same file) moves, which is noise.  The implementation runs diff
+twice and greps, and this can be quite expensive.  To speed things up,
+binary files without textconv filters will be ignored.
 
 When `-S` or `-G` are used without `--pickaxe-all`, only filepairs
 that match their respective criterion are kept in the output.  When
-- 
2.45.2

[PATCH v4 06/10] diff: --patch-{grep,modifies} arg names for -G and -S

[Illia Bobyr]

Most arguments have both short and long versions.  Long versions are
easier to read, especially in scripts and command history.

This change mostly keeps existing uses of -G and -S as is in the tests,
documentation and help output.

Tests that check just the option parsing are duplicated to check both
short and long argument options.

Signed-off-by: Illia Bobyr <illia.bobyr@gmail.com>
---
 Documentation/diff-options.adoc |  2 ++
 Documentation/gitdiffcore.adoc  |  3 ++-
 diff.c                          | 12 ++++++----
 diff.h                          |  8 +++++--
 t/t4209-log-pickaxe.sh          | 42 +++++++++++++++++++++++++++++++++
 5 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc
index 640eb..07413d 100644
--- a/Documentation/diff-options.adoc
+++ b/Documentation/diff-options.adoc
@@ -650,6 +650,7 @@ Note that not all diffs can feature all types. For instance, copied and
 renamed entries cannot appear if detection for those types is disabled.
 
 `-S<string>`::
+`--patch-modifies=<string>`::
 	Look for differences that change the number of occurrences of
 	the specified _<string>_ (i.e. addition/deletion) in a file.
 	Intended for the scripter's use.
@@ -663,6 +664,7 @@ very first version of the block.
 Binary files are searched as well.
 
 `-G<regex>`::
+`--patch-grep=<regex>`::
 	Look for differences whose patch text contains added/removed
 	lines that match _<regex>_.
 +
diff --git a/Documentation/gitdiffcore.adoc b/Documentation/gitdiffcore.adoc
index 0d7d66..e934b9 100644
--- a/Documentation/gitdiffcore.adoc
+++ b/Documentation/gitdiffcore.adoc
@@ -245,7 +245,8 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 
 This transformation limits the set of filepairs to those that change
 specified strings between the preimage and the postimage in a certain
-way.  `-S<string>` and `-G<regex>` options are used to specify
+way.  `--patch-modifies=<string>` (`-S<string>` for short) and
+`--patch-grep=<regex>` (`-G<regex>` for short) are used to specify
 different ways these strings are sought.
 
 `-S<string>` detects filepairs whose preimage and postimage
diff --git a/diff.c b/diff.c
index bd9db..ac2cd 100644
--- a/diff.c
+++ b/diff.c
@@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
 		die(_("options '%s', '%s', and '%s' cannot be used together"),
-			"-G", "-S", "--find-object");
+			"-G/--patch-grep", "-S/--patch-modifies", "--find-object");
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))
 		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s'"),
-			"-G", "--pickaxe-regex", "--pickaxe-regex", "-S");
+			"-G/--patch-grep", "--pickaxe-regex",
+			"--pickaxe-regex", "-S/--patch-modifies");
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))
 		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'"),
-			"--pickaxe-all", "--find-object", "--pickaxe-all", "-G", "-S");
+			"--pickaxe-all", "--find-object",
+			"--pickaxe-all", "-G/--patch-grep", "-S/--patch-modifies");
 
 	/*
 	 * Most of the time we can say "there are changes"
@@ -5862,10 +5864,10 @@ struct option *add_diff_options(const struct option *opts,
 		OPT_SET_INT_F(0, "ita-visible-in-index", &options->ita_invisible_in_index,
 			      N_("treat 'git add -N' entries as real in the index"),
 			      0, PARSE_OPT_NONEG),
-		OPT_CALLBACK_F('S', NULL, options, N_("<string>"),
+		OPT_CALLBACK_F('S', "patch-modifies", options, N_("<string>"),
 			       N_("look for differences that change the number of occurrences of the specified string"),
 			       0, diff_opt_pickaxe_string),
-		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
+		OPT_CALLBACK_F('G', "patch-grep", options, N_("<regex>"),
 			       N_("look for differences where a patch contains the specified regex"),
 			       0, diff_opt_pickaxe_regex),
 		OPT_BIT_F(0, "pickaxe-all", &options->pickaxe_opts,
diff --git a/diff.h b/diff.h
index 787bb..ed48a 100644
--- a/diff.h
+++ b/diff.h
@@ -606,8 +606,12 @@ void diffcore_fix_diff_index(void);
 "                try unchanged files as candidate for copy detection.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
-"  -G<regex>     find differences where patch contains the specified regex.\n" \
-"  -S<string>    find filepair who differ in the number of occurrences of string.\n" \
+"  -G<regex>\n" \
+"  --patch-grep=<regex>\n" \
+"                find differences where patch contains the regex.\n" \
+"  -S<string>\n" \
+"  --patch-modifies=<string>\n" \
+"                find filepair who differ in the number of occurrences of string.\n" \
 "  --pickaxe-grep\n" \
 "                treat <string> as a regex in the -S argument.\n" \
 "  --pickaxe-all\n" \
diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index ed70c..ab14b 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -60,24 +60,48 @@ test_expect_success 'usage' '
 	test_expect_code 129 git log -S 2>err &&
 	test_grep "switch.*requires a value" err &&
 
+	test_expect_code 129 git log --patch-modifies 2>err &&
+	test_grep "option.*requires a value" err &&
+
 	test_expect_code 129 git log -G 2>err &&
 	test_grep "switch.*requires a value" err &&
 
+	test_expect_code 129 git log --patch-grep 2>err &&
+	test_grep "option.*requires a value" err &&
+
 	test_expect_code 128 git log -Gregex -Sstring 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log -Gregex --patch-modifies string 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex -Sstring 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex --patch-modifies string 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log --patch-grep regex --find-object=HEAD 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log --patch-modifies string --find-object=HEAD 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&
 	grep "cannot be used together" err
 '
 
 test_expect_success 'usage: --pickaxe-regex' '
 	test_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex --pickaxe-regex 2>err &&
 	grep "cannot be used together" err
 '
 
@@ -89,7 +113,13 @@ test_expect_success 'usage: --no-pickaxe-regex' '
 	test_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&
 	test_cmp expect actual &&
 
+	test_expect_code 128 git log --patch-modifies string --no-pickaxe-regex 2>actual &&
+	test_cmp expect actual &&
+
 	test_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&
+	test_cmp expect actual &&
+
+	test_expect_code 128 git log --patch-grep regex --no-pickaxe-regex 2>err &&
 	test_cmp expect actual
 '
 
@@ -104,9 +134,13 @@ test_log_icase	expect_second	--author person
 test_log_icase	expect_nomatch	--author spreon
 
 test_log	expect_nomatch	-G picked
+test_log	expect_nomatch	--patch-grep picked
 test_log	expect_second	-G Picked
+test_log	expect_second	--patch-grep Picked
 test_log_icase	expect_nomatch	-G pickle
+test_log_icase	expect_nomatch	--patch-grep pickle
 test_log_icase	expect_second	-G picked
+test_log_icase	expect_second	--patch-grep picked
 
 test_expect_success 'log -G --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
@@ -122,14 +156,22 @@ test_expect_success 'log -G --no-textconv (missing textconv tool)' '
 '
 
 test_log	expect_nomatch	-S picked
+test_log	expect_nomatch	--patch-modifies picked
 test_log	expect_second	-S Picked
+test_log	expect_second	--patch-modifies Picked
 test_log_icase	expect_second	-S picked
+test_log_icase	expect_second	--patch-modifies picked
 test_log_icase	expect_nomatch	-S pickle
+test_log_icase	expect_nomatch	--patch-modifies pickle
 
 test_log	expect_nomatch	-S p.cked --pickaxe-regex
+test_log	expect_nomatch	--patch-modifies p.cked --pickaxe-regex
 test_log	expect_second	-S P.cked --pickaxe-regex
+test_log	expect_second	--patch-modifies P.cked --pickaxe-regex
 test_log_icase	expect_second	-S p.cked --pickaxe-regex
+test_log_icase	expect_second	--patch-modifies p.cked --pickaxe-regex
 test_log_icase	expect_nomatch	-S p.ckle --pickaxe-regex
+test_log_icase	expect_nomatch	--patch-modifies p.ckle --pickaxe-regex
 
 test_expect_success 'log -S --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-- 
2.45.2

[PATCH v4 07/10] completion: Support --patch-{grep,modifies}

[Illia Bobyr]

---
 contrib/completion/git-completion.bash | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 41391..daf335 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1900,6 +1900,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
 			--output= --output-indicator-context=
 			--output-indicator-new= --output-indicator-old=
 			--ws-error-highlight=
+			--patch-grep= --patch-modifies=
 			--pickaxe-all --pickaxe-regex --patch-with-raw
 "
 
@@ -2216,7 +2217,7 @@ __git_complete_log_opts ()
 		__git_complete_symbol --cur="${cur#:}" --sfx=":"
 		return
 		;;
-	-G,*|-S,*)
+	-G,*|--patch-grep,*|-S,*|--patch-modifies,*)
 		__git_complete_symbol
 		return
 		;;
@@ -2239,6 +2240,14 @@ __git_complete_log_opts ()
 		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
 		return
 		;;
+	--patch-grep=*)
+		__git_complete_symbol --pfx="--patch-grep=" --cur="${cur#--patch-grep=}"
+		return
+		;;
+	--patch-modifies=*)
+		__git_complete_symbol --pfx="--patch-modifies=" --cur="${cur#--patch-modifies=}"
+		return
+		;;
 	--submodule=*)
 		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
 		return
-- 
2.45.2

[PATCH v4 08/10] diff: test: Use --patch-{grep,modifies} over -G/-S

[Illia Bobyr]

Long argument names are easier to read, compared to short ones.  So
while short arguments are great when you want to type a command quickly,
tests are more readable if we use long argument names.

There are still test that verify that both short and long arguments work
interchangeably when parsing the arguments.

Tests where the focus is not on the argument names are updated to use
long argument names.
---
 t/t4062-diff-pickaxe.sh |  8 +++---
 t/t4209-log-pickaxe.sh  | 62 ++++++++++++++++++++---------------------
 2 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh
index 8ad3d7..805e0f 100755
--- a/t/t4062-diff-pickaxe.sh
+++ b/t/t4062-diff-pickaxe.sh
@@ -16,13 +16,13 @@ test_expect_success setup '
 '
 
 # OpenBSD only supports up to 255 repetitions, so repeat twice for 64*64=4096.
-test_expect_success '-G matches' '
-	git diff --name-only -G "^(0{64}){64}$" HEAD^ >out &&
+test_expect_success '--patch-grep matches' '
+	git diff --name-only --patch-grep "^(0{64}){64}$" HEAD^ >out &&
 	test 4096-zeroes.txt = "$(cat out)"
 '
 
-test_expect_success '-S --pickaxe-regex' '
-	git diff --name-only -S0 --pickaxe-regex HEAD^ >out &&
+test_expect_success '--patch-modifies --pickaxe-regex' '
+	git diff --name-only --patch-modifies 0 --pickaxe-regex HEAD^ >out &&
 	test 4096-zeroes.txt = "$(cat out)"
 '
 
diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index ab14b..5f4d6 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='log --grep/--author/--regexp-ignore-case/-S/-G'
+test_description='log --grep/--author/--regexp-ignore-case/--patch-{modifies,grep}'
 
 . ./test-lib.sh
 
@@ -142,15 +142,15 @@ test_log_icase	expect_nomatch	--patch-grep pickle
 test_log_icase	expect_second	-G picked
 test_log_icase	expect_second	--patch-grep picked
 
-test_expect_success 'log -G --textconv (missing textconv tool)' '
+test_expect_success 'log --patch-grep --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	test_must_fail git -c diff.test.textconv=missing log -Gfoo &&
+	test_must_fail git -c diff.test.textconv=missing log --patch-grep foo &&
 	rm .gitattributes
 '
 
-test_expect_success 'log -G --no-textconv (missing textconv tool)' '
+test_expect_success 'log --patch-grep --no-textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	git -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&
+	git -c diff.test.textconv=missing log --patch-grep foo --no-textconv >actual &&
 	test_cmp expect_nomatch actual &&
 	rm .gitattributes
 '
@@ -173,20 +173,20 @@ test_log_icase	expect_second	--patch-modifies p.cked --pickaxe-regex
 test_log_icase	expect_nomatch	-S p.ckle --pickaxe-regex
 test_log_icase	expect_nomatch	--patch-modifies p.ckle --pickaxe-regex
 
-test_expect_success 'log -S --textconv (missing textconv tool)' '
+test_expect_success 'log --patch-modifies --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	test_must_fail git -c diff.test.textconv=missing log -Sfoo &&
+	test_must_fail git -c diff.test.textconv=missing log --patch-modifies foo &&
 	rm .gitattributes
 '
 
-test_expect_success 'log -S --no-textconv (missing textconv tool)' '
+test_expect_success 'log --patch-modifies --no-textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	git -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&
+	git -c diff.test.textconv=missing log --patch-modifies foo --no-textconv >actual &&
 	test_cmp expect_nomatch actual &&
 	rm .gitattributes
 '
 
-test_expect_success 'setup log -[GS] plain & regex' '
+test_expect_success 'setup log --patch{-modifies,-grep} plain & regex' '
 	test_create_repo GS-plain &&
 	test_commit -C GS-plain --append A data.txt "a" &&
 	test_commit -C GS-plain --append B data.txt "a a" &&
@@ -201,31 +201,31 @@ test_expect_success 'setup log -[GS] plain & regex' '
 	git -C GS-plain log >full-log
 '
 
-test_expect_success 'log -G trims diff new/old [-+]' '
-	git -C GS-plain log -G"[+-]a" >log &&
+test_expect_success 'log --patch-grep trims diff new/old [-+]' '
+	git -C GS-plain log --patch-grep "[+-]a" >log &&
 	test_must_be_empty log &&
-	git -C GS-plain log -G"^a" >log &&
+	git -C GS-plain log --patch-grep "^a" >log &&
 	test_cmp log A-to-B-then-E-log
 '
 
-test_expect_success 'log -S<pat> is not a regex, but -S<pat> --pickaxe-regex is' '
-	git -C GS-plain log -S"a" >log &&
+test_expect_success 'log --patch-modifies <pat> is not a regex, but --patch-modifies <pat> --pickaxe-regex is' '
+	git -C GS-plain log --patch-modifies "a" >log &&
 	test_cmp log A-to-B-then-E-log &&
 
-	git -C GS-plain log -S"[a]" >log &&
+	git -C GS-plain log --patch-modifies "[a]" >log &&
 	test_must_be_empty log &&
 
-	git -C GS-plain log -S"[a]" --pickaxe-regex >log &&
+	git -C GS-plain log --patch-modifies "[a]" --pickaxe-regex >log &&
 	test_cmp log A-to-B-then-E-log &&
 
-	git -C GS-plain log -S"[b]" >log &&
+	git -C GS-plain log --patch-modifies "[b]" >log &&
 	test_cmp log D-then-E-log &&
 
-	git -C GS-plain log -S"[b]" --pickaxe-regex >log &&
+	git -C GS-plain log --patch-modifies "[b]" --pickaxe-regex >log &&
 	test_cmp log C-to-D-then-E-log
 '
 
-test_expect_success 'setup log -[GS] binary & --text' '
+test_expect_success 'setup log --patch{-modifies,-grep} binary & --text' '
 	test_create_repo GS-bin-txt &&
 	test_commit -C GS-bin-txt --printf A data.bin "a\na\0a\n" &&
 	test_commit -C GS-bin-txt --append --printf B data.bin "a\na\0a\n" &&
@@ -233,36 +233,36 @@ test_expect_success 'setup log -[GS] binary & --text' '
 	git -C GS-bin-txt log >full-log
 '
 
-test_expect_success 'log -G ignores binary files' '
-	git -C GS-bin-txt log -Ga >log &&
+test_expect_success 'log --patch-grep ignores binary files' '
+	git -C GS-bin-txt log --patch-grep a >log &&
 	test_must_be_empty log
 '
 
-test_expect_success 'log -G looks into binary files with -a' '
-	git -C GS-bin-txt log -a -Ga >log &&
+test_expect_success 'log --patch-grep looks into binary files with -a' '
+	git -C GS-bin-txt log -a --patch-grep a >log &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -G looks into binary files with textconv filter' '
+test_expect_success 'log --patch-grep looks into binary files with textconv filter' '
 	test_when_finished "rm GS-bin-txt/.gitattributes" &&
 	(
 		cd GS-bin-txt &&
 		echo "* diff=bin" >.gitattributes &&
-		git -c diff.bin.textconv=cat log -Ga >../log
+		git -c diff.bin.textconv=cat log --patch-grep a >../log
 	) &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -S looks into binary files' '
-	git -C GS-bin-txt log -Sa >log &&
+test_expect_success 'log --patch-modifies looks into binary files' '
+	git -C GS-bin-txt log --patch-modifies a >log &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -S --pickaxe-regex looks into binary files' '
-	git -C GS-bin-txt log --pickaxe-regex -Sa >log &&
+test_expect_success 'log --patch-modifies --pickaxe-regex looks into binary files' '
+	git -C GS-bin-txt log --pickaxe-regex --patch-modifies a >log &&
 	test_cmp log full-log &&
 
-	git -C GS-bin-txt log --pickaxe-regex -S"[a]" >log &&
+	git -C GS-bin-txt log --pickaxe-regex --patch-modifies "[a]" >log &&
 	test_cmp log full-log
 '
 
-- 
2.45.2

[PATCH v4 09/10] diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}

[Illia Bobyr]

For less experienced users --patch-{grep,modifies} should be easier to
understand than just -S or -G.  By mentioning the long argument names in
the help messages we save those users from having to search the list of
options for an explanation of what -S or -G stand for.
---
 diff.c | 4 ++--
 diff.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index ac2cd..a9e78 100644
--- a/diff.c
+++ b/diff.c
@@ -5871,10 +5871,10 @@ struct option *add_diff_options(const struct option *opts,
 			       N_("look for differences where a patch contains the specified regex"),
 			       0, diff_opt_pickaxe_regex),
 		OPT_BIT_F(0, "pickaxe-all", &options->pickaxe_opts,
-			  N_("show all changes in the changeset with -S or -G"),
+			  N_("show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep"),
 			  DIFF_PICKAXE_ALL, PARSE_OPT_NONEG),
 		OPT_BIT_F(0, "pickaxe-regex", &options->pickaxe_opts,
-			  N_("treat <string> in -S as extended POSIX regular expression"),
+			  N_("treat <string> in -S/--patch-modifies as extended POSIX regular expression"),
 			  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),
 		OPT_FILENAME('O', NULL, &options->orderfile,
 			     N_("control the order in which files appear in the output")),
diff --git a/diff.h b/diff.h
index ed48a..9ad37 100644
--- a/diff.h
+++ b/diff.h
@@ -613,9 +613,9 @@ void diffcore_fix_diff_index(void);
 "  --patch-modifies=<string>\n" \
 "                find filepair who differ in the number of occurrences of string.\n" \
 "  --pickaxe-grep\n" \
-"                treat <string> as a regex in the -S argument.\n" \
+"                treat <string> as a regex in the -S/--patch-modifies argument.\n" \
 "  --pickaxe-all\n" \
-"                show all files diff when -G or -S is used and hit is found.\n" \
+"                show all files diff for -G/--patch-grep and -S/--patch-modifies.\n" \
 "  -a  --text    treat all files as text.\n"
 
 int diff_queue_is_empty(struct diff_options *o);
-- 
2.45.2

[PATCH v4 10/10] diff: docs: Use --patch-{grep,modifies} over -G/-S

[Illia Bobyr]

Long argument names are easier to read, compared to short ones.  So
while short arguments are great when you want to type a command quickly,
the documentation readability is improved if we use long argument names.

Note for reviewers:  All changes are just a replacement of `-G` with
`--patch-grep` and `-S` with `--patch-modifies`.  But as the text was
reformatted to fit the same width in a few places it might look like
there are more changes, if the diff is only line-wise and not word-wise.

The only an exception are changes in `gitdiffcore.adoc`, where I did
rephrase a sentence.  I've moved introduction of the short versions of
the `--patch-{grep,modifies}` into a subsequent paragraph.  The reason
is that I wanted to keep a note on the `-G` mnemonic, and it was awkward
if I would repeat the short definition twice over a span of two
paragraphs.
---
 Documentation/diff-options.adoc | 34 ++++++++++----------
 Documentation/git-blame.adoc    |  2 +-
 Documentation/gitdiffcore.adoc  | 55 +++++++++++++++++----------------
 3 files changed, 46 insertions(+), 45 deletions(-)

diff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc
index 07413d..c9f7c 100644
--- a/Documentation/diff-options.adoc
+++ b/Documentation/diff-options.adoc
@@ -658,8 +658,8 @@ renamed entries cannot appear if detection for those types is disabled.
 It is useful when you're looking for an exact block of code (like a
 struct), and want to know the history of that block since it first
 came into being: use the feature iteratively to feed the interesting
-block in the preimage back into `-S`, and keep going until you get the
-very first version of the block.
+block in the preimage back into `--patch-modifies`, and keep going until
+you get the very first version of the block.
 +
 Binary files are searched as well.
 
@@ -668,9 +668,9 @@ Binary files are searched as well.
 	Look for differences whose patch text contains added/removed
 	lines that match _<regex>_.
 +
-To illustrate the difference between `-S<regex>` `--pickaxe-regex` and
-`-G<regex>`, consider a commit with the following diff in the same
-file:
+To illustrate the difference between `--patch-modifies=<regex>
+--pickaxe-regex` and `--patch-grep=<regex>`, consider a commit with the
+following diff in the same file:
 +
 ----
 +    return frotz(nitfol, two->ptr, 1, 0);
@@ -678,9 +678,9 @@ file:
 -    hit = frotz(nitfol, mf2.ptr, 1, 0);
 ----
 +
-While `git log -G"frotz\(nitfol"` will show this commit, `git log
--S"frotz\(nitfol" --pickaxe-regex` will not (because the number of
-occurrences of that string did not change).
+While `git log --patch-grep="frotz\(nitfol"` will show this commit, `git
+log --patch-modifies="frotz\(nitfol" --pickaxe-regex` will not (because the
+number of occurrences of that string did not change).
 +
 Unless `--text` is supplied patches of binary files without a textconv
 filter will be ignored.
@@ -689,22 +689,22 @@ See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more
 information.
 
 `--find-object=<object-id>`::
-	Look for differences that change the number of occurrences of
-	the specified object. Similar to `-S`, just the argument is different
-	in that it doesn't search for a specific string but for a specific
-	object id.
+	Look for differences that change the number of occurrences of the
+	specified object. Similar to `--patch-modifies`, just the argument
+	is different in that it doesn't search for a specific string but
+	for a specific object id.
 +
 The object can be a blob or a submodule commit. It implies the `-t` option in
 `git-log` to also find trees.
 
 `--pickaxe-all`::
-	When `-S` or `-G` finds a change, show all the changes in that
-	changeset, not just the files that contain the change
-	in _<string>_.
+	When `--patch-modifies` or `--patch-grep` finds a change, show all
+	the changes in that changeset, not just the files that contain the
+	change in _<string>_.
 
 `--pickaxe-regex`::
-	Treat the _<string>_ given to `-S` as an extended POSIX regular
-	expression to match.
+	Treat the _<string>_ given to `--patch-modifies` as an extended
+	POSIX regular expression to match.
 
 endif::git-format-patch[]
 
diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
index f75ed..10736a 100644
--- a/Documentation/git-blame.adoc
+++ b/Documentation/git-blame.adoc
@@ -41,7 +41,7 @@ a text string in the diff. A small example of the pickaxe interface
 that searches for `blame_usage`:
 
 -----------------------------------------------------------------------------
-$ git log --pretty=oneline -S'blame_usage'
+$ git log --pretty=oneline --patch-modifies='blame_usage'
 5040f17eba15504bad66b14a645bddd9b015ebb7 blame -S <ancestry-file>
 ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output
 -----------------------------------------------------------------------------
diff --git a/Documentation/gitdiffcore.adoc b/Documentation/gitdiffcore.adoc
index e934b9..e7f98 100644
--- a/Documentation/gitdiffcore.adoc
+++ b/Documentation/gitdiffcore.adoc
@@ -245,33 +245,34 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 
 This transformation limits the set of filepairs to those that change
 specified strings between the preimage and the postimage in a certain
-way.  `--patch-modifies=<string>` (`-S<string>` for short) and
-`--patch-grep=<regex>` (`-G<regex>` for short) are used to specify
-different ways these strings are sought.
-
-`-S<string>` detects filepairs whose preimage and postimage
-have different number of occurrences of the specified _<string>_.
-By definition, it will not detect in-file moves.  Also, when a
-changeset moves a file wholesale without affecting the interesting
-string, diffcore-rename kicks in as usual, and `-S` omits the filepair
-(since the number of occurrences of that string didn't change in that
-rename-detected filepair).  When used with `--pickaxe-regex`, treat
-the _<string>_ as an extended POSIX regular expression to match,
-instead of a literal string.
-
-`-G<regex>` (mnemonic: grep) detects filepairs whose textual diff has
-an added or a deleted line that matches the given _<regex>_.  This
-means that it will detect in-file (or what rename-detection considers
-the same file) moves, which is noise.  The implementation runs diff
-twice and greps, and this can be quite expensive.  To speed things up,
-binary files without textconv filters will be ignored.
-
-When `-S` or `-G` are used without `--pickaxe-all`, only filepairs
-that match their respective criterion are kept in the output.  When
-`--pickaxe-all` is used, if even one filepair matches their respective
-criterion in a changeset, the entire changeset is kept.  This behavior
-is designed to make reviewing changes in the context of the whole
-changeset easier.
+way.  `--patch-modifies=<string>` and `--patch-grep=<regex>` are used
+to specify different ways these strings are sought.
+
+`--patch-modifies=<string>` (`-S<string>` for short) detects filepairs
+whose preimage and postimage have different number of occurrences of
+the specified _<string>_.  By definition, it will not detect in-file
+moves.  Also, when a changeset moves a file wholesale without
+affecting the interesting string, diffcore-rename kicks in as usual,
+and `--patch-modifies` omits the filepair (since the number of
+occurrences of that string didn't change in that rename-detected
+filepair).  When used with `--pickaxe-regex`, treat the _<string>_ as
+an extended POSIX regular expression to match, instead of a literal
+string.
+
+`--patch-grep=<regex>` (`-G<regex>` for short, mnemonic: grep) detects
+filepairs whose textual diff has an added or a deleted line that
+matches the given regular expression.  This means that it will detect
+in-file (or what rename-detection considers the same file) moves,
+which is noise.  The implementation runs diff twice and greps, and
+this can be quite expensive.  To speed things up, binary files without
+textconv filters will be ignored.
+
+When `--patch-modifies` or `--patch-grep` are used without
+`--pickaxe-all`, only filepairs that match their respective criterion
+are kept in the output.  When `--pickaxe-all` is used, if even one
+filepair matches their respective criterion in a changeset, the entire
+changeset is kept.  This behavior is designed to make reviewing
+changes in the context of the whole changeset easier.
 
 diffcore-order: For Sorting the Output Based on Filenames
 ---------------------------------------------------------
-- 
2.45.2

[PATCH v5 00/10] Long names for `git log -S` and `git log -G`

[Illia Bobyr]

Rebased v4 [1] on top of `master`.
I've accidentally published v4 based on `next`.

[1] https://lore.kernel.org/git/20250211085028.3923875-1-illia.bobyr@gmail.com/

Other than the rebase, it is identical to v4, details below.

Split the big change from v3 [1] into multiple, mostly independent patches, to
make it easier to review and merge each one separately.

Patches 1 through 4 are fixing minor bugs and inconsistencies.

Patch 5 contains updates gitdiffcore to use same placeholder names as the rest
of the code.

Patch 6 contains a minimum change to add long versions of -S and -G.

Patch 7 adds bash completion support.

Patches 8 through 10 increase usage of the long argument versions in tests, CLI
help and docs respectively.

Please, let me know if you prefer it split in a different way, or reorder the
changes.

Illia Bobyr (10):
  t/t4209-log-pickaxe: Naming typo: -G takes a regex
  diff: -G description: Correct copy/paste error
  diff: short help: Correct -S description
  diff: short help: Add -G and --pickaxe-grep
  docs: gitdiffcore: -G and -S: Use regex/string placeholders
  diff: --patch-{grep,modifies} arg names for -G and -S
  completion: Support --patch-{grep,modifies}
  diff: test: Use --patch-{grep,modifies} over -G/-S
  diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}
  diff: docs: Use --patch-{grep,modifies} over -G/-S

 Documentation/diff-options.txt         |  36 +++++----
 Documentation/git-blame.txt            |   2 +-
 Documentation/gitdiffcore.txt          |  55 ++++++-------
 contrib/completion/git-completion.bash |  11 ++-
 diff.c                                 |  18 +++--
 diff.h                                 |  11 ++-
 t/t4062-diff-pickaxe.sh                |   8 +-
 t/t4209-log-pickaxe.sh                 | 106 +++++++++++++++++--------
 8 files changed, 155 insertions(+), 92 deletions(-)

-- 
2.45.2

[PATCH v5 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex

[Illia Bobyr]

Not effect on the test logic, but as "-G" argument is a regex it is more
accurate to use "regex" as a dummy argument value rather than "string".
In all the other case when "-G" is passed a dummy value it is spelled as
"regex" rather than as "string".
---
 t/t4209-log-pickaxe.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index a675ac..ed70c 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -89,7 +89,7 @@ test_expect_success 'usage: --no-pickaxe-regex' '
 	test_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&
 	test_cmp expect actual &&
 
-	test_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&
+	test_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&
 	test_cmp expect actual
 '
 
-- 
2.45.2

Re: [PATCH v5 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> Subject: Re: [PATCH v5 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex

"Naming" -> "naming".  It is even more preferable if you can avoid
introducing the second colon.  E.g.

    Subject: t/t4209: call the value given to -G <regex>, not <string>

This applies to the titles of all the patches in the series, I suspect?

> Not effect on the test logic, but as "-G" argument is a regex it is more
> accurate to use "regex" as a dummy argument value rather than "string".
> In all the other case when "-G" is passed a dummy value it is spelled as
> "regex" rather than as "string".

I guess the -G tests are copied-and-pasted from existing tests for
the -S option when the -G option was introduced much later.  This
makes me wonder if we try to see what happens when a malformed
regular expression is fed to the -G option (I didn't check---if we
have no test for it, we might want to add one).

> ---

Missing sign-off.

Having said all that, I'd prefer to see the changes in one topic
focused only to give --patch-grep and --patch-modifies synonyms
to the existing option and do nothing else.  Certainly a change
like this is a distraction we do not have to discuss at the same
time.

If you can group all these "preliminary clean-up" changes together
into a separate series, without including any change to add the
longhand to -S/-G, that is also a viable alternative approach.  Once
such a series graduates to 'master', then you'd do the longhand on
top.

What we do not want is to see changes that are not directly
necessary to add the longhand intermixed in the same series.

>  t/t4209-log-pickaxe.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
> index a675ac..ed70c 100755
> --- a/t/t4209-log-pickaxe.sh
> +++ b/t/t4209-log-pickaxe.sh
> @@ -89,7 +89,7 @@ test_expect_success 'usage: --no-pickaxe-regex' '
>  	test_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&
>  	test_cmp expect actual &&
>  
> -	test_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&
> +	test_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&
>  	test_cmp expect actual
>  '

Thanks.

[PATCH v5 02/10] diff: -G description: Correct copy/paste error

[Illia Bobyr]

Current description for -G is incorrect, seems like it was copied from
the description for -S.
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 019fb..bd9db 100644
--- a/diff.c
+++ b/diff.c
@@ -5866,7 +5866,7 @@ struct option *add_diff_options(const struct option *opts,
 			       N_("look for differences that change the number of occurrences of the specified string"),
 			       0, diff_opt_pickaxe_string),
 		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
-			       N_("look for differences that change the number of occurrences of the specified regex"),
+			       N_("look for differences where a patch contains the specified regex"),
 			       0, diff_opt_pickaxe_regex),
 		OPT_BIT_F(0, "pickaxe-all", &options->pickaxe_opts,
 			  N_("show all changes in the changeset with -S or -G"),
-- 
2.45.2

Re: [PATCH v5 02/10] diff: -G description: Correct copy/paste error

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> Current description for -G is incorrect, seems like it was copied from
> the description for -S.
> ---
>  diff.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

[jc: I won't point out about the title and sign-off, which are the
same issues [1/10] had and maybe shared with the later patches]

>
> diff --git a/diff.c b/diff.c
> index 019fb..bd9db 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5866,7 +5866,7 @@ struct option *add_diff_options(const struct option *opts,
>  			       N_("look for differences that change the number of occurrences of the specified string"),
>  			       0, diff_opt_pickaxe_string),
>  		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
> -			       N_("look for differences that change the number of occurrences of the specified regex"),
> +			       N_("look for differences where a patch contains the specified regex"),

Yeah, but the updated one is not all that great, either.  -S looks
for string, so either "occurences" or "contains" would work, but
a patch that "contains" the regular expression would not necessarily
match with -G ;-) 

"a patch contains a line that matches" is closer but not correct.

What the option looks for is if there is a changed line in the patch
that matches the given regular expression.  If a context line shared
between the preimage and the postimage matches the regular expression
that does not count as a "hit".

    N_("find differences with changed lines that match the given regex").

perhaps?

Again, this is "preliminary clean-up" (or "after-the-dust-settles")
material and shouldn't be part of the main series.

Thanks.

[PATCH v5 03/10] diff: short help: Correct -S description

[Illia Bobyr]

`-S` shows changes that modify the number of occurrences of the
specified string, rather than only those that either completely remove
it or add it for the first time.
---
 diff.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diff.h b/diff.h
index 0a566f..49ece3 100644
--- a/diff.h
+++ b/diff.h
@@ -606,7 +606,7 @@ void diffcore_fix_diff_index(void);
 "                try unchanged files as candidate for copy detection.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
-"  -S<string>    find filepair whose only one side contains the string.\n" \
+"  -S<string>    find filepair who differ in the number of occurrences of string.\n" \
 "  --pickaxe-all\n" \
 "                show all files diff when -S is used and hit is found.\n" \
 "  -a  --text    treat all files as text.\n"
-- 
2.45.2

Re: [PATCH v5 03/10] diff: short help: Correct -S description

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> `-S` shows changes that modify the number of occurrences of the
> specified string, rather than only those that either completely remove
> it or add it for the first time.
> ---

[jc: title, sign-off, and this should be done outside the main topic
are shared with other patches, so I won't repeat them]

>  diff.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/diff.h b/diff.h
> index 0a566f..49ece3 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -606,7 +606,7 @@ void diffcore_fix_diff_index(void);
>  "                try unchanged files as candidate for copy detection.\n" \
>  "  -l<n>         limit rename attempts up to <n> paths.\n" \
>  "  -O<file>      reorder diffs according to the <file>.\n" \
> -"  -S<string>    find filepair whose only one side contains the string.\n" \
> +"  -S<string>    find filepair who differ in the number of occurrences of string.\n" \

Given that the initial motivation of -S<block-of-text> was to find
"before it wasn't there in that shape, now there it is" [*1*] (and
<block-of-text> wasn meant to be something unique in the codebase),
the original conveys the intent better, but the updated text
describes the actual behaviour more correctly (in other words, the
behaviour of the option when a not-so-unique string that may produce
multiple hits is given is whatever it happens to do---which is "see
the number of occurrences are different").

So this is a good change.


[Foornote]

 *1* The original motivation of "-S<block-of-text>" was to be a
     building block for the tool Linus dreamed in his message
     https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org
     that explained why recording renames at the commit time is a
     bad idea.


>  "  --pickaxe-all\n" \
>  "                show all files diff when -S is used and hit is found.\n" \
>  "  -a  --text    treat all files as text.\n"

[PATCH v5 04/10] diff: short help: Add -G and --pickaxe-grep

[Illia Bobyr]

-G and --pickaxe-grep seems to be on par with -S and --pickaxe-all that
are already mentioned.
---
 diff.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/diff.h b/diff.h
index 49ece3..787bb 100644
--- a/diff.h
+++ b/diff.h
@@ -606,9 +606,12 @@ void diffcore_fix_diff_index(void);
 "                try unchanged files as candidate for copy detection.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
+"  -G<regex>     find differences where patch contains the specified regex.\n" \
 "  -S<string>    find filepair who differ in the number of occurrences of string.\n" \
+"  --pickaxe-grep\n" \
+"                treat <string> as a regex in the -S argument.\n" \
 "  --pickaxe-all\n" \
-"                show all files diff when -S is used and hit is found.\n" \
+"                show all files diff when -G or -S is used and hit is found.\n" \
 "  -a  --text    treat all files as text.\n"
 
 int diff_queue_is_empty(struct diff_options *o);
-- 
2.45.2

[PATCH v5 05/10] docs: gitdiffcore: -G and -S: Use regex/string placeholders

[Illia Bobyr]

In the rest of the documentation (and in the code) we use `regex` and
`string` as `-G` and `-S` argument placeholders.  While
`regular-expression` and `block-of-text` are a bit easier to read, it is
a bit consistent.

And we could assume that everyone who uses git should be able to
understand that a "string" and a "block-of-text", as well as a "regex"
and "regular-expression" are the same thing.  So, using a shorter
version is also more consistent.
---
 Documentation/gitdiffcore.txt | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index 642c5..0d7d66 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -245,26 +245,25 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 
 This transformation limits the set of filepairs to those that change
 specified strings between the preimage and the postimage in a certain
-way.  -S<block-of-text> and -G<regular-expression> options are used to
-specify different ways these strings are sought.
+way.  `-S<string>` and `-G<regex>` options are used to specify
+different ways these strings are sought.
 
-"-S<block-of-text>" detects filepairs whose preimage and postimage
-have different number of occurrences of the specified block of text.
+`-S<string>` detects filepairs whose preimage and postimage
+have different number of occurrences of the specified _<string>_.
 By definition, it will not detect in-file moves.  Also, when a
 changeset moves a file wholesale without affecting the interesting
 string, diffcore-rename kicks in as usual, and `-S` omits the filepair
 (since the number of occurrences of that string didn't change in that
 rename-detected filepair).  When used with `--pickaxe-regex`, treat
-the <block-of-text> as an extended POSIX regular expression to match,
+the _<string>_ as an extended POSIX regular expression to match,
 instead of a literal string.
 
-"-G<regular-expression>" (mnemonic: grep) detects filepairs whose
-textual diff has an added or a deleted line that matches the given
-regular expression.  This means that it will detect in-file (or what
-rename-detection considers the same file) moves, which is noise.  The
-implementation runs diff twice and greps, and this can be quite
-expensive.  To speed things up, binary files without textconv filters
-will be ignored.
+`-G<regex>` (mnemonic: grep) detects filepairs whose textual diff has
+an added or a deleted line that matches the given _<regex>_.  This
+means that it will detect in-file (or what rename-detection considers
+the same file) moves, which is noise.  The implementation runs diff
+twice and greps, and this can be quite expensive.  To speed things up,
+binary files without textconv filters will be ignored.
 
 When `-S` or `-G` are used without `--pickaxe-all`, only filepairs
 that match their respective criterion are kept in the output.  When
-- 
2.45.2

Re: [PATCH v5 05/10] docs: gitdiffcore: -G and -S: Use regex/string placeholders

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> In the rest of the documentation (and in the code) we use `regex` and
> `string` as `-G` and `-S` argument placeholders.  While
> `regular-expression` and `block-of-text` are a bit easier to read, it is
> a bit consistent.
>
> And we could assume that everyone who uses git should be able to
> understand that a "string" and a "block-of-text", as well as a "regex"
> and "regular-expression" are the same thing.  So, using a shorter
> version is also more consistent.
> ---
>  Documentation/gitdiffcore.txt | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)

I am with one reservation.  <block-of-text> was written to stress
the fact that most of the time the string is expected to be a
multi-line block of text (like a function body) [*1*] that is unique
within the codebase.  I do not think replacing a short-and-sweet
<string> with <block-of-text> is a good idea, but if we are to go
this route, we should mention that to compensate for the diminished
stress on that block-ness of the text, as <string> would imply
something quite short and would fit on a single line.

[Footnote]

*1* Go back to Linus's message I cited earlier;
    https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/

[PATCH v5 06/10] diff: --patch-{grep,modifies} arg names for -G and -S

[Illia Bobyr]

Most arguments have both short and long versions.  Long versions are
easier to read, especially in scripts and command history.

This change mostly keeps existing uses of -G and -S as is in the tests,
documentation and help output.

Tests that check just the option parsing are duplicated to check both
short and long argument options.

Signed-off-by: Illia Bobyr <illia.bobyr@gmail.com>
---
 Documentation/diff-options.txt |  2 ++
 Documentation/gitdiffcore.txt  |  3 ++-
 diff.c                         | 12 ++++++----
 diff.h                         |  8 +++++--
 t/t4209-log-pickaxe.sh         | 42 ++++++++++++++++++++++++++++++++++
 5 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 640eb..07413d 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -650,6 +650,7 @@ Note that not all diffs can feature all types. For instance, copied and
 renamed entries cannot appear if detection for those types is disabled.
 
 `-S<string>`::
+`--patch-modifies=<string>`::
 	Look for differences that change the number of occurrences of
 	the specified _<string>_ (i.e. addition/deletion) in a file.
 	Intended for the scripter's use.
@@ -663,6 +664,7 @@ very first version of the block.
 Binary files are searched as well.
 
 `-G<regex>`::
+`--patch-grep=<regex>`::
 	Look for differences whose patch text contains added/removed
 	lines that match _<regex>_.
 +
diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index 0d7d66..e934b9 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -245,7 +245,8 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 
 This transformation limits the set of filepairs to those that change
 specified strings between the preimage and the postimage in a certain
-way.  `-S<string>` and `-G<regex>` options are used to specify
+way.  `--patch-modifies=<string>` (`-S<string>` for short) and
+`--patch-grep=<regex>` (`-G<regex>` for short) are used to specify
 different ways these strings are sought.
 
 `-S<string>` detects filepairs whose preimage and postimage
diff --git a/diff.c b/diff.c
index bd9db..ac2cd 100644
--- a/diff.c
+++ b/diff.c
@@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
 		die(_("options '%s', '%s', and '%s' cannot be used together"),
-			"-G", "-S", "--find-object");
+			"-G/--patch-grep", "-S/--patch-modifies", "--find-object");
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))
 		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s'"),
-			"-G", "--pickaxe-regex", "--pickaxe-regex", "-S");
+			"-G/--patch-grep", "--pickaxe-regex",
+			"--pickaxe-regex", "-S/--patch-modifies");
 
 	if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))
 		die(_("options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'"),
-			"--pickaxe-all", "--find-object", "--pickaxe-all", "-G", "-S");
+			"--pickaxe-all", "--find-object",
+			"--pickaxe-all", "-G/--patch-grep", "-S/--patch-modifies");
 
 	/*
 	 * Most of the time we can say "there are changes"
@@ -5862,10 +5864,10 @@ struct option *add_diff_options(const struct option *opts,
 		OPT_SET_INT_F(0, "ita-visible-in-index", &options->ita_invisible_in_index,
 			      N_("treat 'git add -N' entries as real in the index"),
 			      0, PARSE_OPT_NONEG),
-		OPT_CALLBACK_F('S', NULL, options, N_("<string>"),
+		OPT_CALLBACK_F('S', "patch-modifies", options, N_("<string>"),
 			       N_("look for differences that change the number of occurrences of the specified string"),
 			       0, diff_opt_pickaxe_string),
-		OPT_CALLBACK_F('G', NULL, options, N_("<regex>"),
+		OPT_CALLBACK_F('G', "patch-grep", options, N_("<regex>"),
 			       N_("look for differences where a patch contains the specified regex"),
 			       0, diff_opt_pickaxe_regex),
 		OPT_BIT_F(0, "pickaxe-all", &options->pickaxe_opts,
diff --git a/diff.h b/diff.h
index 787bb..ed48a 100644
--- a/diff.h
+++ b/diff.h
@@ -606,8 +606,12 @@ void diffcore_fix_diff_index(void);
 "                try unchanged files as candidate for copy detection.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
-"  -G<regex>     find differences where patch contains the specified regex.\n" \
-"  -S<string>    find filepair who differ in the number of occurrences of string.\n" \
+"  -G<regex>\n" \
+"  --patch-grep=<regex>\n" \
+"                find differences where patch contains the regex.\n" \
+"  -S<string>\n" \
+"  --patch-modifies=<string>\n" \
+"                find filepair who differ in the number of occurrences of string.\n" \
 "  --pickaxe-grep\n" \
 "                treat <string> as a regex in the -S argument.\n" \
 "  --pickaxe-all\n" \
diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index ed70c..ab14b 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -60,24 +60,48 @@ test_expect_success 'usage' '
 	test_expect_code 129 git log -S 2>err &&
 	test_grep "switch.*requires a value" err &&
 
+	test_expect_code 129 git log --patch-modifies 2>err &&
+	test_grep "option.*requires a value" err &&
+
 	test_expect_code 129 git log -G 2>err &&
 	test_grep "switch.*requires a value" err &&
 
+	test_expect_code 129 git log --patch-grep 2>err &&
+	test_grep "option.*requires a value" err &&
+
 	test_expect_code 128 git log -Gregex -Sstring 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log -Gregex --patch-modifies string 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex -Sstring 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex --patch-modifies string 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log --patch-grep regex --find-object=HEAD 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&
 	grep "cannot be used together" err &&
 
+	test_expect_code 128 git log --patch-modifies string --find-object=HEAD 2>err &&
+	grep "cannot be used together" err &&
+
 	test_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&
 	grep "cannot be used together" err
 '
 
 test_expect_success 'usage: --pickaxe-regex' '
 	test_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&
+	grep "cannot be used together" err &&
+
+	test_expect_code 128 git log --patch-grep regex --pickaxe-regex 2>err &&
 	grep "cannot be used together" err
 '
 
@@ -89,7 +113,13 @@ test_expect_success 'usage: --no-pickaxe-regex' '
 	test_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&
 	test_cmp expect actual &&
 
+	test_expect_code 128 git log --patch-modifies string --no-pickaxe-regex 2>actual &&
+	test_cmp expect actual &&
+
 	test_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&
+	test_cmp expect actual &&
+
+	test_expect_code 128 git log --patch-grep regex --no-pickaxe-regex 2>err &&
 	test_cmp expect actual
 '
 
@@ -104,9 +134,13 @@ test_log_icase	expect_second	--author person
 test_log_icase	expect_nomatch	--author spreon
 
 test_log	expect_nomatch	-G picked
+test_log	expect_nomatch	--patch-grep picked
 test_log	expect_second	-G Picked
+test_log	expect_second	--patch-grep Picked
 test_log_icase	expect_nomatch	-G pickle
+test_log_icase	expect_nomatch	--patch-grep pickle
 test_log_icase	expect_second	-G picked
+test_log_icase	expect_second	--patch-grep picked
 
 test_expect_success 'log -G --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
@@ -122,14 +156,22 @@ test_expect_success 'log -G --no-textconv (missing textconv tool)' '
 '
 
 test_log	expect_nomatch	-S picked
+test_log	expect_nomatch	--patch-modifies picked
 test_log	expect_second	-S Picked
+test_log	expect_second	--patch-modifies Picked
 test_log_icase	expect_second	-S picked
+test_log_icase	expect_second	--patch-modifies picked
 test_log_icase	expect_nomatch	-S pickle
+test_log_icase	expect_nomatch	--patch-modifies pickle
 
 test_log	expect_nomatch	-S p.cked --pickaxe-regex
+test_log	expect_nomatch	--patch-modifies p.cked --pickaxe-regex
 test_log	expect_second	-S P.cked --pickaxe-regex
+test_log	expect_second	--patch-modifies P.cked --pickaxe-regex
 test_log_icase	expect_second	-S p.cked --pickaxe-regex
+test_log_icase	expect_second	--patch-modifies p.cked --pickaxe-regex
 test_log_icase	expect_nomatch	-S p.ckle --pickaxe-regex
+test_log_icase	expect_nomatch	--patch-modifies p.ckle --pickaxe-regex
 
 test_expect_success 'log -S --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-- 
2.45.2

Re: [PATCH v5 06/10] diff: --patch-{grep,modifies} arg names for -G and -S

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> Most arguments have both short and long versions.  Long versions are
> easier to read, especially in scripts and command history.
>
> This change mostly keeps existing uses of -G and -S as is in the tests,
> documentation and help output.
>
> Tests that check just the option parsing are duplicated to check both
> short and long argument options.
>
> Signed-off-by: Illia Bobyr <illia.bobyr@gmail.com>
> ---

This step looks mostly good, but the option descriptions for
existing "-S" and "-G" have been touched by clean-up changes
earlier, so they cannot be separated out.

We can treat [01-05/10] as a separate 5-patch "preliminary
clean-up" series and discard this and later steps until the earlier
half lands.

Unlike other earlier steps, this one has a decent title and it has
your sign-off, both of which are good.  And of course it is very
much on topic.

[PATCH v5 07/10] completion: Support --patch-{grep,modifies}

[Illia Bobyr]

---
 contrib/completion/git-completion.bash | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 41391..daf335 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1900,6 +1900,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
 			--output= --output-indicator-context=
 			--output-indicator-new= --output-indicator-old=
 			--ws-error-highlight=
+			--patch-grep= --patch-modifies=
 			--pickaxe-all --pickaxe-regex --patch-with-raw
 "
 
@@ -2216,7 +2217,7 @@ __git_complete_log_opts ()
 		__git_complete_symbol --cur="${cur#:}" --sfx=":"
 		return
 		;;
-	-G,*|-S,*)
+	-G,*|--patch-grep,*|-S,*|--patch-modifies,*)
 		__git_complete_symbol
 		return
 		;;
@@ -2239,6 +2240,14 @@ __git_complete_log_opts ()
 		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
 		return
 		;;
+	--patch-grep=*)
+		__git_complete_symbol --pfx="--patch-grep=" --cur="${cur#--patch-grep=}"
+		return
+		;;
+	--patch-modifies=*)
+		__git_complete_symbol --pfx="--patch-modifies=" --cur="${cur#--patch-modifies=}"
+		return
+		;;
 	--submodule=*)
 		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
 		return
-- 
2.45.2

Re: [PATCH v5 07/10] completion: Support --patch-{grep,modifies}

[Junio C Hamano]

Illia Bobyr <illia.bobyr@gmail.com> writes:

> Subject: Re: [PATCH v5 07/10] completion: Support --patch-{grep,modifies}

"Support" -> "support".


> ---
>  contrib/completion/git-completion.bash | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

Missing sign-off.

Other than that, this is a great material to be part of the main
topic to add the longhands to these two options.

Thanks.


> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 41391..daf335 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1900,6 +1900,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
>  			--output= --output-indicator-context=
>  			--output-indicator-new= --output-indicator-old=
>  			--ws-error-highlight=
> +			--patch-grep= --patch-modifies=
>  			--pickaxe-all --pickaxe-regex --patch-with-raw
>  "
>  
> @@ -2216,7 +2217,7 @@ __git_complete_log_opts ()
>  		__git_complete_symbol --cur="${cur#:}" --sfx=":"
>  		return
>  		;;
> -	-G,*|-S,*)
> +	-G,*|--patch-grep,*|-S,*|--patch-modifies,*)
>  		__git_complete_symbol
>  		return
>  		;;
> @@ -2239,6 +2240,14 @@ __git_complete_log_opts ()
>  		__gitcomp "$__git_diff_algorithms" "" "${cur##--diff-algorithm=}"
>  		return
>  		;;
> +	--patch-grep=*)
> +		__git_complete_symbol --pfx="--patch-grep=" --cur="${cur#--patch-grep=}"
> +		return
> +		;;
> +	--patch-modifies=*)
> +		__git_complete_symbol --pfx="--patch-modifies=" --cur="${cur#--patch-modifies=}"
> +		return
> +		;;
>  	--submodule=*)
>  		__gitcomp "$__git_diff_submodule_formats" "" "${cur##--submodule=}"
>  		return

[PATCH v5 08/10] diff: test: Use --patch-{grep,modifies} over -G/-S

[Illia Bobyr]

Long argument names are easier to read, compared to short ones.  So
while short arguments are great when you want to type a command quickly,
tests are more readable if we use long argument names.

There are still test that verify that both short and long arguments work
interchangeably when parsing the arguments.

Tests where the focus is not on the argument names are updated to use
long argument names.
---
 t/t4062-diff-pickaxe.sh |  8 +++---
 t/t4209-log-pickaxe.sh  | 62 ++++++++++++++++++++---------------------
 2 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh
index 8ad3d7..805e0f 100755
--- a/t/t4062-diff-pickaxe.sh
+++ b/t/t4062-diff-pickaxe.sh
@@ -16,13 +16,13 @@ test_expect_success setup '
 '
 
 # OpenBSD only supports up to 255 repetitions, so repeat twice for 64*64=4096.
-test_expect_success '-G matches' '
-	git diff --name-only -G "^(0{64}){64}$" HEAD^ >out &&
+test_expect_success '--patch-grep matches' '
+	git diff --name-only --patch-grep "^(0{64}){64}$" HEAD^ >out &&
 	test 4096-zeroes.txt = "$(cat out)"
 '
 
-test_expect_success '-S --pickaxe-regex' '
-	git diff --name-only -S0 --pickaxe-regex HEAD^ >out &&
+test_expect_success '--patch-modifies --pickaxe-regex' '
+	git diff --name-only --patch-modifies 0 --pickaxe-regex HEAD^ >out &&
 	test 4096-zeroes.txt = "$(cat out)"
 '
 
diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh
index ab14b..5f4d6 100755
--- a/t/t4209-log-pickaxe.sh
+++ b/t/t4209-log-pickaxe.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='log --grep/--author/--regexp-ignore-case/-S/-G'
+test_description='log --grep/--author/--regexp-ignore-case/--patch-{modifies,grep}'
 
 . ./test-lib.sh
 
@@ -142,15 +142,15 @@ test_log_icase	expect_nomatch	--patch-grep pickle
 test_log_icase	expect_second	-G picked
 test_log_icase	expect_second	--patch-grep picked
 
-test_expect_success 'log -G --textconv (missing textconv tool)' '
+test_expect_success 'log --patch-grep --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	test_must_fail git -c diff.test.textconv=missing log -Gfoo &&
+	test_must_fail git -c diff.test.textconv=missing log --patch-grep foo &&
 	rm .gitattributes
 '
 
-test_expect_success 'log -G --no-textconv (missing textconv tool)' '
+test_expect_success 'log --patch-grep --no-textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	git -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&
+	git -c diff.test.textconv=missing log --patch-grep foo --no-textconv >actual &&
 	test_cmp expect_nomatch actual &&
 	rm .gitattributes
 '
@@ -173,20 +173,20 @@ test_log_icase	expect_second	--patch-modifies p.cked --pickaxe-regex
 test_log_icase	expect_nomatch	-S p.ckle --pickaxe-regex
 test_log_icase	expect_nomatch	--patch-modifies p.ckle --pickaxe-regex
 
-test_expect_success 'log -S --textconv (missing textconv tool)' '
+test_expect_success 'log --patch-modifies --textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	test_must_fail git -c diff.test.textconv=missing log -Sfoo &&
+	test_must_fail git -c diff.test.textconv=missing log --patch-modifies foo &&
 	rm .gitattributes
 '
 
-test_expect_success 'log -S --no-textconv (missing textconv tool)' '
+test_expect_success 'log --patch-modifies --no-textconv (missing textconv tool)' '
 	echo "* diff=test" >.gitattributes &&
-	git -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&
+	git -c diff.test.textconv=missing log --patch-modifies foo --no-textconv >actual &&
 	test_cmp expect_nomatch actual &&
 	rm .gitattributes
 '
 
-test_expect_success 'setup log -[GS] plain & regex' '
+test_expect_success 'setup log --patch{-modifies,-grep} plain & regex' '
 	test_create_repo GS-plain &&
 	test_commit -C GS-plain --append A data.txt "a" &&
 	test_commit -C GS-plain --append B data.txt "a a" &&
@@ -201,31 +201,31 @@ test_expect_success 'setup log -[GS] plain & regex' '
 	git -C GS-plain log >full-log
 '
 
-test_expect_success 'log -G trims diff new/old [-+]' '
-	git -C GS-plain log -G"[+-]a" >log &&
+test_expect_success 'log --patch-grep trims diff new/old [-+]' '
+	git -C GS-plain log --patch-grep "[+-]a" >log &&
 	test_must_be_empty log &&
-	git -C GS-plain log -G"^a" >log &&
+	git -C GS-plain log --patch-grep "^a" >log &&
 	test_cmp log A-to-B-then-E-log
 '
 
-test_expect_success 'log -S<pat> is not a regex, but -S<pat> --pickaxe-regex is' '
-	git -C GS-plain log -S"a" >log &&
+test_expect_success 'log --patch-modifies <pat> is not a regex, but --patch-modifies <pat> --pickaxe-regex is' '
+	git -C GS-plain log --patch-modifies "a" >log &&
 	test_cmp log A-to-B-then-E-log &&
 
-	git -C GS-plain log -S"[a]" >log &&
+	git -C GS-plain log --patch-modifies "[a]" >log &&
 	test_must_be_empty log &&
 
-	git -C GS-plain log -S"[a]" --pickaxe-regex >log &&
+	git -C GS-plain log --patch-modifies "[a]" --pickaxe-regex >log &&
 	test_cmp log A-to-B-then-E-log &&
 
-	git -C GS-plain log -S"[b]" >log &&
+	git -C GS-plain log --patch-modifies "[b]" >log &&
 	test_cmp log D-then-E-log &&
 
-	git -C GS-plain log -S"[b]" --pickaxe-regex >log &&
+	git -C GS-plain log --patch-modifies "[b]" --pickaxe-regex >log &&
 	test_cmp log C-to-D-then-E-log
 '
 
-test_expect_success 'setup log -[GS] binary & --text' '
+test_expect_success 'setup log --patch{-modifies,-grep} binary & --text' '
 	test_create_repo GS-bin-txt &&
 	test_commit -C GS-bin-txt --printf A data.bin "a\na\0a\n" &&
 	test_commit -C GS-bin-txt --append --printf B data.bin "a\na\0a\n" &&
@@ -233,36 +233,36 @@ test_expect_success 'setup log -[GS] binary & --text' '
 	git -C GS-bin-txt log >full-log
 '
 
-test_expect_success 'log -G ignores binary files' '
-	git -C GS-bin-txt log -Ga >log &&
+test_expect_success 'log --patch-grep ignores binary files' '
+	git -C GS-bin-txt log --patch-grep a >log &&
 	test_must_be_empty log
 '
 
-test_expect_success 'log -G looks into binary files with -a' '
-	git -C GS-bin-txt log -a -Ga >log &&
+test_expect_success 'log --patch-grep looks into binary files with -a' '
+	git -C GS-bin-txt log -a --patch-grep a >log &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -G looks into binary files with textconv filter' '
+test_expect_success 'log --patch-grep looks into binary files with textconv filter' '
 	test_when_finished "rm GS-bin-txt/.gitattributes" &&
 	(
 		cd GS-bin-txt &&
 		echo "* diff=bin" >.gitattributes &&
-		git -c diff.bin.textconv=cat log -Ga >../log
+		git -c diff.bin.textconv=cat log --patch-grep a >../log
 	) &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -S looks into binary files' '
-	git -C GS-bin-txt log -Sa >log &&
+test_expect_success 'log --patch-modifies looks into binary files' '
+	git -C GS-bin-txt log --patch-modifies a >log &&
 	test_cmp log full-log
 '
 
-test_expect_success 'log -S --pickaxe-regex looks into binary files' '
-	git -C GS-bin-txt log --pickaxe-regex -Sa >log &&
+test_expect_success 'log --patch-modifies --pickaxe-regex looks into binary files' '
+	git -C GS-bin-txt log --pickaxe-regex --patch-modifies a >log &&
 	test_cmp log full-log &&
 
-	git -C GS-bin-txt log --pickaxe-regex -S"[a]" >log &&
+	git -C GS-bin-txt log --pickaxe-regex --patch-modifies "[a]" >log &&
 	test_cmp log full-log
 '
 
-- 
2.45.2

[PATCH v5 09/10] diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}

[Illia Bobyr]

For less experienced users --patch-{grep,modifies} should be easier to
understand than just -S or -G.  By mentioning the long argument names in
the help messages we save those users from having to search the list of
options for an explanation of what -S or -G stand for.
---
 diff.c | 4 ++--
 diff.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index ac2cd..a9e78 100644
--- a/diff.c
+++ b/diff.c
@@ -5871,10 +5871,10 @@ struct option *add_diff_options(const struct option *opts,
 			       N_("look for differences where a patch contains the specified regex"),
 			       0, diff_opt_pickaxe_regex),
 		OPT_BIT_F(0, "pickaxe-all", &options->pickaxe_opts,
-			  N_("show all changes in the changeset with -S or -G"),
+			  N_("show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep"),
 			  DIFF_PICKAXE_ALL, PARSE_OPT_NONEG),
 		OPT_BIT_F(0, "pickaxe-regex", &options->pickaxe_opts,
-			  N_("treat <string> in -S as extended POSIX regular expression"),
+			  N_("treat <string> in -S/--patch-modifies as extended POSIX regular expression"),
 			  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),
 		OPT_FILENAME('O', NULL, &options->orderfile,
 			     N_("control the order in which files appear in the output")),
diff --git a/diff.h b/diff.h
index ed48a..9ad37 100644
--- a/diff.h
+++ b/diff.h
@@ -613,9 +613,9 @@ void diffcore_fix_diff_index(void);
 "  --patch-modifies=<string>\n" \
 "                find filepair who differ in the number of occurrences of string.\n" \
 "  --pickaxe-grep\n" \
-"                treat <string> as a regex in the -S argument.\n" \
+"                treat <string> as a regex in the -S/--patch-modifies argument.\n" \
 "  --pickaxe-all\n" \
-"                show all files diff when -G or -S is used and hit is found.\n" \
+"                show all files diff for -G/--patch-grep and -S/--patch-modifies.\n" \
 "  -a  --text    treat all files as text.\n"
 
 int diff_queue_is_empty(struct diff_options *o);
-- 
2.45.2

[PATCH v5 10/10] diff: docs: Use --patch-{grep,modifies} over -G/-S

[Illia Bobyr]

Long argument names are easier to read, compared to short ones.  So
while short arguments are great when you want to type a command quickly,
the documentation readability is improved if we use long argument names.

Note for reviewers:  All changes are just a replacement of `-G` with
`--patch-grep` and `-S` with `--patch-modifies`.  But as the text was
reformatted to fit the same width in a few places it might look like
there are more changes, if the diff is only line-wise and not word-wise.

The only an exception are changes in `gitdiffcore.adoc`, where I did
rephrase a sentence.  I've moved introduction of the short versions of
the `--patch-{grep,modifies}` into a subsequent paragraph.  The reason
is that I wanted to keep a note on the `-G` mnemonic, and it was awkward
if I would repeat the short definition twice over a span of two
paragraphs.
---
 Documentation/diff-options.txt | 34 ++++++++++-----------
 Documentation/git-blame.txt    |  2 +-
 Documentation/gitdiffcore.txt  | 55 +++++++++++++++++-----------------
 3 files changed, 46 insertions(+), 45 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 07413d..c9f7c 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -658,8 +658,8 @@ renamed entries cannot appear if detection for those types is disabled.
 It is useful when you're looking for an exact block of code (like a
 struct), and want to know the history of that block since it first
 came into being: use the feature iteratively to feed the interesting
-block in the preimage back into `-S`, and keep going until you get the
-very first version of the block.
+block in the preimage back into `--patch-modifies`, and keep going until
+you get the very first version of the block.
 +
 Binary files are searched as well.
 
@@ -668,9 +668,9 @@ Binary files are searched as well.
 	Look for differences whose patch text contains added/removed
 	lines that match _<regex>_.
 +
-To illustrate the difference between `-S<regex>` `--pickaxe-regex` and
-`-G<regex>`, consider a commit with the following diff in the same
-file:
+To illustrate the difference between `--patch-modifies=<regex>
+--pickaxe-regex` and `--patch-grep=<regex>`, consider a commit with the
+following diff in the same file:
 +
 ----
 +    return frotz(nitfol, two->ptr, 1, 0);
@@ -678,9 +678,9 @@ file:
 -    hit = frotz(nitfol, mf2.ptr, 1, 0);
 ----
 +
-While `git log -G"frotz\(nitfol"` will show this commit, `git log
--S"frotz\(nitfol" --pickaxe-regex` will not (because the number of
-occurrences of that string did not change).
+While `git log --patch-grep="frotz\(nitfol"` will show this commit, `git
+log --patch-modifies="frotz\(nitfol" --pickaxe-regex` will not (because the
+number of occurrences of that string did not change).
 +
 Unless `--text` is supplied patches of binary files without a textconv
 filter will be ignored.
@@ -689,22 +689,22 @@ See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more
 information.
 
 `--find-object=<object-id>`::
-	Look for differences that change the number of occurrences of
-	the specified object. Similar to `-S`, just the argument is different
-	in that it doesn't search for a specific string but for a specific
-	object id.
+	Look for differences that change the number of occurrences of the
+	specified object. Similar to `--patch-modifies`, just the argument
+	is different in that it doesn't search for a specific string but
+	for a specific object id.
 +
 The object can be a blob or a submodule commit. It implies the `-t` option in
 `git-log` to also find trees.
 
 `--pickaxe-all`::
-	When `-S` or `-G` finds a change, show all the changes in that
-	changeset, not just the files that contain the change
-	in _<string>_.
+	When `--patch-modifies` or `--patch-grep` finds a change, show all
+	the changes in that changeset, not just the files that contain the
+	change in _<string>_.
 
 `--pickaxe-regex`::
-	Treat the _<string>_ given to `-S` as an extended POSIX regular
-	expression to match.
+	Treat the _<string>_ given to `--patch-modifies` as an extended
+	POSIX regular expression to match.
 
 endif::git-format-patch[]
 
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index b1d7fb..0f21d3 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -41,7 +41,7 @@ a text string in the diff. A small example of the pickaxe interface
 that searches for `blame_usage`:
 
 -----------------------------------------------------------------------------
-$ git log --pretty=oneline -S'blame_usage'
+$ git log --pretty=oneline --patch-modifies='blame_usage'
 5040f17eba15504bad66b14a645bddd9b015ebb7 blame -S <ancestry-file>
 ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output
 -----------------------------------------------------------------------------
diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
index e934b9..e7f98 100644
--- a/Documentation/gitdiffcore.txt
+++ b/Documentation/gitdiffcore.txt
@@ -245,33 +245,34 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
 
 This transformation limits the set of filepairs to those that change
 specified strings between the preimage and the postimage in a certain
-way.  `--patch-modifies=<string>` (`-S<string>` for short) and
-`--patch-grep=<regex>` (`-G<regex>` for short) are used to specify
-different ways these strings are sought.
-
-`-S<string>` detects filepairs whose preimage and postimage
-have different number of occurrences of the specified _<string>_.
-By definition, it will not detect in-file moves.  Also, when a
-changeset moves a file wholesale without affecting the interesting
-string, diffcore-rename kicks in as usual, and `-S` omits the filepair
-(since the number of occurrences of that string didn't change in that
-rename-detected filepair).  When used with `--pickaxe-regex`, treat
-the _<string>_ as an extended POSIX regular expression to match,
-instead of a literal string.
-
-`-G<regex>` (mnemonic: grep) detects filepairs whose textual diff has
-an added or a deleted line that matches the given _<regex>_.  This
-means that it will detect in-file (or what rename-detection considers
-the same file) moves, which is noise.  The implementation runs diff
-twice and greps, and this can be quite expensive.  To speed things up,
-binary files without textconv filters will be ignored.
-
-When `-S` or `-G` are used without `--pickaxe-all`, only filepairs
-that match their respective criterion are kept in the output.  When
-`--pickaxe-all` is used, if even one filepair matches their respective
-criterion in a changeset, the entire changeset is kept.  This behavior
-is designed to make reviewing changes in the context of the whole
-changeset easier.
+way.  `--patch-modifies=<string>` and `--patch-grep=<regex>` are used
+to specify different ways these strings are sought.
+
+`--patch-modifies=<string>` (`-S<string>` for short) detects filepairs
+whose preimage and postimage have different number of occurrences of
+the specified _<string>_.  By definition, it will not detect in-file
+moves.  Also, when a changeset moves a file wholesale without
+affecting the interesting string, diffcore-rename kicks in as usual,
+and `--patch-modifies` omits the filepair (since the number of
+occurrences of that string didn't change in that rename-detected
+filepair).  When used with `--pickaxe-regex`, treat the _<string>_ as
+an extended POSIX regular expression to match, instead of a literal
+string.
+
+`--patch-grep=<regex>` (`-G<regex>` for short, mnemonic: grep) detects
+filepairs whose textual diff has an added or a deleted line that
+matches the given regular expression.  This means that it will detect
+in-file (or what rename-detection considers the same file) moves,
+which is noise.  The implementation runs diff twice and greps, and
+this can be quite expensive.  To speed things up, binary files without
+textconv filters will be ignored.
+
+When `--patch-modifies` or `--patch-grep` are used without
+`--pickaxe-all`, only filepairs that match their respective criterion
+are kept in the output.  When `--pickaxe-all` is used, if even one
+filepair matches their respective criterion in a changeset, the entire
+changeset is kept.  This behavior is designed to make reviewing
+changes in the context of the whole changeset easier.
 
 diffcore-order: For Sorting the Output Based on Filenames
 ---------------------------------------------------------
-- 
2.45.2