Long names for `git log -S` and `git log -G`

Long names for `git log -S` and `git log -G`

[Illia Bobyr]

Hello,

I would like to add long names for the `-S` and `-G` options for `git log`.
It seems that most options have long versions, though there are some 
exceptions.

I was wondering if there could be any objections.
And also, what would be a good name for each.

Both are provided by the diff-pickaxe functionality.
`-S` is already affected by `--pickaxe-regex` and both `-G` and `-S` are 
affected by `--pickaxe-all`.

Also,`diffcore` docs says:

 > "-G<regular-expression>" (mnemonic: grep)

I was thinking of `--pickaxe` for `-S` and `--grep` for `-G`.
And it would probably make sense to discuss this before I try submitting 
a patch.

`--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
Not sure what would be a reasonably short alternative for `-S`.
`--pickaxe-occurance-change` seems too long, and might not be as clear.
`--pickaxe-occurance-count-change` is just way too long.

Thank you,
Illia Bobyr

Re: Long names for `git log -S` and `git log -G`

[Junio C Hamano]

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

> Also,`diffcore` docs says:
>
>> "-G<regular-expression>" (mnemonic: grep)
>
> I was thinking of `--pickaxe` for `-S` and `--grep` for `-G`.

In the context of "git diff", calling "-G" "--grep" would be OK, but
in the context of "git log", there is "--grep" already, so that
won't fly.

> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.

That is probably OK (even though "-G" is not exactly what the
pickaxe machinery wants to do; "--grep-in-patch" might be closer to
the intent).

> Not sure what would be a reasonably short alternative for `-S`.
> `--pickaxe-occurance-change` seems too long, and might not be as clear.
> `--pickaxe-occurance-count-change` is just way too long.

Giving a tool a meaningful name is an excellent idea.  If the
meaningful name guides users to the right way to use the tool,
it would be ideal.  Which means that to name it right, you'd need to
know what it exactly is for.

The -S feature was written to become one of the building blocks of
Linus's "clearly superior algorithm", described in [1].  Linus talks
about "where did this _line_ come from?", but the algorithm is more
generally about a block of code.  The expected use case is for -S to
be fed sufficiently unique block of text so that we can efficiently
detect the transition of occurence count from 1 (because wee start
from sufficiently unique block of code) down to 0 (which is the
boundary in history where the block of code was first introduced in
its current form).  It detects any occurence count change, but its
primary focus is to find a transition from 1 to 0 (when going
backwards in history).  Its spirit is more about "finding where it
appeared in its current shape".


[Footnote]

*1* https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/

Re: Long names for `git log -S` and `git log -G`

[Jeff King]

On Tue, Nov 19, 2024 at 12:52:50PM +0900, Junio C Hamano wrote:

> 
> > `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
> 
> That is probably OK (even though "-G" is not exactly what the
> pickaxe machinery wants to do; "--grep-in-patch" might be closer to
> the intent).

FWIW, I like --grep-in-patch. Saying just "--pickaxe-grep" does not
highlight that it is about looking in the patch. I.e., it is not clear
from the name that is different from "-Sfoo --pickaxe-regex".

> The -S feature was written to become one of the building blocks of
> Linus's "clearly superior algorithm", described in [1].  Linus talks
> about "where did this _line_ come from?", but the algorithm is more
> generally about a block of code.  The expected use case is for -S to
> be fed sufficiently unique block of text so that we can efficiently
> detect the transition of occurence count from 1 (because wee start
> from sufficiently unique block of code) down to 0 (which is the
> boundary in history where the block of code was first introduced in
> its current form).  It detects any occurence count change, but its
> primary focus is to find a transition from 1 to 0 (when going
> backwards in history).  Its spirit is more about "finding where it
> appeared in its current shape".

Heh. I do not disagree that was the original focus, but I find that I
use "-S" most frequently to find mentions of a particular function or
other token across the code base. E.g., understanding the historical
uses of a function, or why it has no callers anymore, and so on. So I am
often looking for many appearances or disappearances (though of course
they may or may not happen in a single file).

-Peff

Re: Long names for `git log -S` and `git log -G`

[Illia Bobyr]

On 11/18/24 19:52, Junio C Hamano wrote:
>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
> That is probably OK (even though "-G" is not exactly what the
> pickaxe machinery wants to do; "--grep-in-patch" might be closer to
> the intent).
Imagining, that I am starting from scratch for this functionality, I think I
would also consider "patch".  Though, as we have 4 related argument names, I
wonder if using it as a prefix would create a more consistent UX.

Something like:

"--patch-grep" for "-G"
"--patch-modifies" for "-S"
"--patch-search-show-all"/"--patch-show-all" for "--pickaxe-all"
"--patch-search-regex"/"--patch-regex" for "--pickaxe-regex"

I'm not too sure about the later two, and also they already have long names.
But it seems quite easy to understand what this command would do:

   git log --patch-grep sometext ...

Or this

   git log --patch-modifies function_call --patch-search-regex ...

>> Not sure what would be a reasonably short alternative for `-S`.
>> `--pickaxe-occurance-change` seems too long, and might not be as clear.
>> `--pickaxe-occurance-count-change` is just way too long.
> Giving a tool a meaningful name is an excellent idea.  If the
> meaningful name guides users to the right way to use the tool,
> it would be ideal.  Which means that to name it right, you'd need to
> know what it exactly is for.

Glad we are on the same page here :)
I would really appreciate yours and Jeff input.
I wrote a simple patch to see how much work is it to add long names [1].
But I would change it based on whatever names are agreed upon.

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

> The -S feature was written to become one of the building blocks of
> Linus's "clearly superior algorithm", described in [1].  Linus talks
> about "where did this _line_ come from?", but the algorithm is more
> generally about a block of code.  The expected use case is for -S to
> be fed sufficiently unique block of text so that we can efficiently
> detect the transition of occurence count from 1 (because wee start
> from sufficiently unique block of code) down to 0 (which is the
> boundary in history where the block of code was first introduced in
> its current form).  It detects any occurence count change, but its
> primary focus is to find a transition from 1 to 0 (when going
> backwards in history).  Its spirit is more about "finding where it
> appeared in its current shape".
>
>
> [Footnote]
>
> *1* https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/

This is pretty interesting.  Thank you for pointing it out.  I guess, it 
means
that the "counting" in the porposed long name of "-S" arguments is more 
of an
implementation detail than the actually intended functionality.  Do you 
think
there is a word that better reflects the intent here? "--patch-modifies" is
probably also not really hitting it 100%.

On 11/19/24 10:58, Jeff King wrote:
> On Tue, Nov 19, 2024 at 12:52:50PM +0900, Junio C Hamano wrote:
>>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
>> That is probably OK (even though "-G" is not exactly what the
>> pickaxe machinery wants to do; "--grep-in-patch" might be closer to
>> the intent).
> FWIW, I like --grep-in-patch. Saying just "--pickaxe-grep" does not
> highlight that it is about looking in the patch. I.e., it is not clear
> from the name that is different from "-Sfoo --pickaxe-regex".

I agree.  Do you think "--patch-grep" is a better name?  Or do you think 
that
the grep and the occurrence counting functionality should not share a common
prefix, as they are somewhat different in nature?

"git log" docs talk about both "-S" and "-G" as if they are pretty 
close.  There
is a note in "git diffcore" doc, "diff-pickaxe" section that says that 
grep is
actually quite different.  I wonder if this is important for the users 
to think
about.  I often use "-G" followed by "-S" or the other way around, just 
to see
which view is better for my particular problem.

Re: Long names for `git log -S` and `git log -G`

[Junio C Hamano]

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

> On 11/18/24 19:52, Junio C Hamano wrote:
>>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
>> That is probably OK (even though "-G" is not exactly what the
>> pickaxe machinery wants to do; "--grep-in-patch" might be closer to
>> the intent).
> Imagining, that I am starting from scratch for this functionality, I think I
> would also consider "patch".  Though, as we have 4 related argument names, I
> wonder if using it as a prefix would create a more consistent UX.
>
> Something like:
>
> "--patch-grep" for "-G"
> "--patch-modifies" for "-S"

Ahh, "modifies" is a great verb.  It sounds quite logical, but "-S"
does not have to genereate a patch internally for it to work, so
"--patch-modifies" is a bit of white lie.

> "--patch-search-show-all"/"--patch-show-all" for "--pickaxe-all"
> "--patch-search-regex"/"--patch-regex" for "--pickaxe-regex"

These already have their own established long names, so it is
outside the scope of this topic, and I doubt it is worth giving
these additional aliases (as you seem to agree).

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

[Illia Bobyr]

Sorry, I dropped the ball on this.
I still think it would be good to get it done.

> On 11/22/24 02:51, Junio C Hamano wrote:
> >>> `--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
> >> That is probably OK (even though "-G" is not exactly what the
> >> pickaxe machinery wants to do; "--grep-in-patch" might be closer to
> >> the intent).
> > Imagining, that I am starting from scratch for this functionality, I think I
> > would also consider "patch".  Though, as we have 4 related argument names, I
> > wonder if using it as a prefix would create a more consistent UX.
> >
> > Something like:
> >
> > "--patch-grep" for "-G"
> > "--patch-modifies" for "-S"
>
> Ahh, "modifies" is a great verb.  It sounds quite logical, but "-S"
> does not have to generate a patch internally for it to work, so
> "--patch-modifies" is a bit of white lie.

Used "--patch-modifies" for the second version of the patch.

I think most people (certainly me) think about patches first, when they look at
these commands.  They are unlikely to realize right away that generating a patch
is more expensive than counting the string/regex occurrences in the pre- and
post- images.  So, if you consider the "it behaves as if" point of view rather
than "this is how it works" point of view, it is not even a lie :)

> > "--patch-search-show-all"/"--patch-show-all" for "--pickaxe-all"
> > "--patch-search-regex"/"--patch-regex" for "--pickaxe-regex"
>
> These already have their own established long names, so it is
> outside the scope of this topic, and I doubt it is worth giving
> these additional aliases (as you seem to agree).

I do not have a strong feeling on this one.  If "-S" and/or "-G" would get names
that do not start with a "--pickaxe" it might be a bit confusing that the flags
that affect their behavior do have the "--pickaxe" prefix.  If this is a valid
concern, I could probably create a separate patch to add alternative names.

---

I've updated the patch with the following names:

"--patch-grep" for "-G"
"--patch-modifies" for "-S"

On 11/19/24 10:58, Jeff King wrote:
> FWIW, I like --grep-in-patch. Saying just "--pickaxe-grep" does not
> highlight that it is about looking in the patch. I.e., it is not clear
> from the name that is different from "-Sfoo --pickaxe-regex".

Is "--patch-grep" a good alternative?  I think, using the same prefix for a
functionality that looks quite similar from the user standpoint ("-G" and "-S")
seems nice.  Using "--grep-in-patch" for "-G", "--patch-modifies" for "-S" and
"--pickaxe-regex"/"--pickaxe-all" all at the same time seems less consistent,
but I can change it if you insist.

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 +++-
 gitk-git/gitk                  |  10 +++-
 t/t4062-diff-pickaxe.sh        |   8 +--
 t/t4209-log-pickaxe.sh         | 106 +++++++++++++++++++++++----------
 8 files changed, 151 insertions(+), 88 deletions(-)

-- 
2.45.2

[PATCH v2 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 +++-
 gitk-git/gitk                  |  10 +++-
 t/t4062-diff-pickaxe.sh        |   8 +--
 t/t4209-log-pickaxe.sh         | 106 +++++++++++++++++++++++----------
 8 files changed, 151 insertions(+), 88 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/gitk-git/gitk b/gitk-git/gitk
index 47a7c1..52516 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -228,7 +228,15 @@ proc parseviewargs {n arglist} {
             "--until=*" - "--before=*" - "--max-age=*" - "--min-age=*" -
             "--author=*" - "--committer=*" - "--grep=*" - "-[iE]" -
             "--remove-empty" - "--first-parent" - "--cherry-pick" -
-            "-S*" - "-G*" - "--pickaxe-all" - "--pickaxe-regex" -
+            "-S*" - "--patch-modifies=*" -
+            "--patch-modifies" {
+              set nextisval 1
+            }
+            "-G*" - "--patch-grep=*" -
+            "--patch-grep" {
+              set nextisval 1
+            }
+            "--pickaxe-all" - "--pickaxe-regex" -
             "--simplify-by-decoration" {
                 # These mean that we get a subset of the commits
                 set filtered 1
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 v2 1/1] diff: --patch{-modifies,grep} arg names for -S and -G

[Johannes Sixt]

Am 05.02.25 um 03:24 schrieb Illia Bobyr:
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 47a7c1..52516 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk

Please exclude this part related to gitk from this submission and send a
follow-up patch with only this part. The subject line should then start
with "gitk:".

-- Hannes