{"thread":{"id":"62909","subject":"[PATCH v3 0/1] Long names for `git log -S` and `git log -G`","startedAt":"2025-02-06T01:43:40Z","lastAt":"2025-02-13T05:46:19Z","messageCount":36,"participants":["Illia Bobyr","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":1},"messages":[{"id":"511918","messageId":"20250206014324.1839232-1-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":null,"subject":"[PATCH v3 0/1] Long names for `git log -S` and `git log -G`","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-06T01:43:15Z","receivedAt":"2025-02-06T01:43:40Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Same as PATCH v2[1], but removed gitk changes, as suggested by Johannes Sixt.\n\nI'll send a separate patch for gitk, should this patch be accepted.\nOr, I could include gitk changes into this chain, but just as a separate patch?\n\n[1]: https://lore.kernel.org/git/20250205022422.2019929-1-illia.bobyr@gmail.com/\n\nIllia Bobyr (1):\n diff: --patch-{modifies,grep} arg names for -S and -G\n\n Documentation/diff-options.txt | 36 +++++------\n Documentation/git-blame.txt | 2 +-\n Documentation/gitdiffcore.txt | 48 ++++++++-------\n diff.c | 18 +++---\n diff.h | 11 +++-\n t/t4062-diff-pickaxe.sh | 8 +--\n t/t4209-log-pickaxe.sh | 106 +++++++++++++++++++++++----------\n 7 files changed, 142 insertions(+), 87 deletions(-)\n\n-- \n2.45.2\n\n"},{"id":"511919","messageId":"20250206014324.1839232-2-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-06T01:43:16Z","receivedAt":"2025-02-06T01:43:43Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Most arguments have both short and long versions. Long versions are\neasier to read, especially in scripts and command history.\n\nTests that check just the option parsing are duplicated to check both\nshort and long argument options. But more complex tests are updated to\nuse the long argument in order to improve the test readability.\nAssuming that the usage tests have already verified that both arguments\ninvoke the same underlying functionality.\n\nSigned-off-by: Illia Bobyr \n---\n Documentation/diff-options.txt | 36 +++++------\n Documentation/git-blame.txt | 2 +-\n Documentation/gitdiffcore.txt | 48 ++++++++-------\n diff.c | 18 +++---\n diff.h | 11 +++-\n t/t4062-diff-pickaxe.sh | 8 +--\n t/t4209-log-pickaxe.sh | 106 +++++++++++++++++++++++----------\n 7 files changed, 142 insertions(+), 87 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 640eb..c9f7c 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -650,6 +650,7 @@ Note that not all diffs can feature all types. For instance, copied and\n renamed entries cannot appear if detection for those types is disabled.\n \n `-S`::\n+`--patch-modifies=`::\n \tLook for differences that change the number of occurrences of\n \tthe specified __ (i.e. addition/deletion) in a file.\n \tIntended for the scripter's use.\n@@ -657,18 +658,19 @@ renamed entries cannot appear if detection for those types is disabled.\n It is useful when you're looking for an exact block of code (like a\n struct), and want to know the history of that block since it first\n came into being: use the feature iteratively to feed the interesting\n-block in the preimage back into `-S`, and keep going until you get the\n-very first version of the block.\n+block in the preimage back into `--patch-modifies`, and keep going until\n+you get the very first version of the block.\n +\n Binary files are searched as well.\n \n `-G`::\n+`--patch-grep=`::\n \tLook for differences whose patch text contains added/removed\n \tlines that match __.\n +\n-To illustrate the difference between `-S` `--pickaxe-regex` and\n-`-G`, consider a commit with the following diff in the same\n-file:\n+To illustrate the difference between `--patch-modifies=\n+--pickaxe-regex` and `--patch-grep=`, consider a commit with the\n+following diff in the same file:\n +\n ----\n + return frotz(nitfol, two->ptr, 1, 0);\n@@ -676,9 +678,9 @@ file:\n - hit = frotz(nitfol, mf2.ptr, 1, 0);\n ----\n +\n-While `git log -G\"frotz\\(nitfol\"` will show this commit, `git log\n--S\"frotz\\(nitfol\" --pickaxe-regex` will not (because the number of\n-occurrences of that string did not change).\n+While `git log --patch-grep=\"frotz\\(nitfol\"` will show this commit, `git\n+log --patch-modifies=\"frotz\\(nitfol\" --pickaxe-regex` will not (because the\n+number of occurrences of that string did not change).\n +\n Unless `--text` is supplied patches of binary files without a textconv\n filter will be ignored.\n@@ -687,22 +689,22 @@ See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more\n information.\n \n `--find-object=`::\n-\tLook for differences that change the number of occurrences of\n-\tthe specified object. Similar to `-S`, just the argument is different\n-\tin that it doesn't search for a specific string but for a specific\n-\tobject id.\n+\tLook for differences that change the number of occurrences of the\n+\tspecified object. Similar to `--patch-modifies`, just the argument\n+\tis different in that it doesn't search for a specific string but\n+\tfor a specific object id.\n +\n The object can be a blob or a submodule commit. It implies the `-t` option in\n `git-log` to also find trees.\n \n `--pickaxe-all`::\n-\tWhen `-S` or `-G` finds a change, show all the changes in that\n-\tchangeset, not just the files that contain the change\n-\tin __.\n+\tWhen `--patch-modifies` or `--patch-grep` finds a change, show all\n+\tthe changes in that changeset, not just the files that contain the\n+\tchange in __.\n \n `--pickaxe-regex`::\n-\tTreat the __ given to `-S` as an extended POSIX regular\n-\texpression to match.\n+\tTreat the __ given to `--patch-modifies` as an extended\n+\tPOSIX regular expression to match.\n \n endif::git-format-patch[]\n \ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex b1d7fb..0f21d3 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -41,7 +41,7 @@ a text string in the diff. A small example of the pickaxe interface\n that searches for `blame_usage`:\n \n -----------------------------------------------------------------------------\n-$ git log --pretty=oneline -S'blame_usage'\n+$ git log --pretty=oneline --patch-modifies='blame_usage'\n 5040f17eba15504bad66b14a645bddd9b015ebb7 blame -S \n ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output\n -----------------------------------------------------------------------------\ndiff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\nindex 642c5..e4b18 100644\n--- a/Documentation/gitdiffcore.txt\n+++ b/Documentation/gitdiffcore.txt\n@@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. -S and -G options are used to\n-specify different ways these strings are sought.\n-\n-\"-S\" detects filepairs whose preimage and postimage\n-have different number of occurrences of the specified block of text.\n-By definition, it will not detect in-file moves. Also, when a\n-changeset moves a file wholesale without affecting the interesting\n-string, diffcore-rename kicks in as usual, and `-S` omits the filepair\n-(since the number of occurrences of that string didn't change in that\n+way. --patch-modifies= and\n+--patch-grep= options are used to specify\n+different ways these strings are sought.\n+\n+\"-S\", or \"--patch-modifies=\" detects\n+filepairs whose preimage and postimage have different number of\n+occurrences of the specified block of text. By definition, it will\n+not detect in-file moves. Also, when a changeset moves a file\n+wholesale without affecting the interesting string, diffcore-rename\n+kicks in as usual, and `--patch-modifies` omits the filepair (since\n+the number of occurrences of that string didn't change in that\n rename-detected filepair). When used with `--pickaxe-regex`, treat\n the as an extended POSIX regular expression to match,\n instead of a literal string.\n \n-\"-G\" (mnemonic: grep) detects filepairs whose\n-textual diff has an added or a deleted line that matches the given\n-regular expression. This means that it will detect in-file (or what\n-rename-detection considers the same file) moves, which is noise. The\n-implementation runs diff twice and greps, and this can be quite\n-expensive. To speed things up, binary files without textconv filters\n-will be ignored.\n-\n-When `-S` or `-G` are used without `--pickaxe-all`, only filepairs\n-that match their respective criterion are kept in the output. When\n-`--pickaxe-all` is used, if even one filepair matches their respective\n-criterion in a changeset, the entire changeset is kept. This behavior\n-is designed to make reviewing changes in the context of the whole\n-changeset easier.\n+\"-G\", or \"--patch-grep=\"\n+(mnemonic: grep) detects filepairs whose textual diff has an added or\n+a deleted line that matches the given regular expression. This means\n+that it will detect in-file (or what rename-detection considers the\n+same file) moves, which is noise. The implementation runs diff twice\n+and greps, and this can be quite expensive. To speed things up,\n+binary files without textconv filters will be ignored.\n+\n+When `--patch-modifies` or `--patch-grep` are used without\n+`--pickaxe-all`, only filepairs that match their respective criterion\n+are kept in the output. When `--pickaxe-all` is used, if even one\n+filepair matches their respective criterion in a changeset, the entire\n+changeset is kept. This behavior is designed to make reviewing\n+changes in the context of the whole changeset easier.\n \n diffcore-order: For Sorting the Output Based on Filenames\n ---------------------------------------------------------\ndiff --git a/diff.c b/diff.c\nindex d28b41..09beb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n-\t\t\t\"-G\", \"-S\", \"--find-object\");\n+\t\t\t\"-G/--patch-grep\", \"-S/--patch-modifies\", \"--find-object\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s'\"),\n-\t\t\t\"-G\", \"--pickaxe-regex\", \"--pickaxe-regex\", \"-S\");\n+\t\t\t\"-G/--patch-grep\", \"--pickaxe-regex\",\n+ \"--pickaxe-regex\", \"-S/--patch-modifies\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'\"),\n-\t\t\t\"--pickaxe-all\", \"--find-object\", \"--pickaxe-all\", \"-G\", \"-S\");\n+\t\t\t\"--pickaxe-all\", \"--find-object\",\n+ \"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\n \n \t/*\n \t * Most of the time we can say \"there are changes\"\n@@ -5862,17 +5864,17 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_SET_INT_F(0, \"ita-visible-in-index\", &options->ita_invisible_in_index,\n \t\t\t N_(\"treat 'git add -N' entries as real in the index\"),\n \t\t\t 0, PARSE_OPT_NONEG),\n-\t\tOPT_CALLBACK_F('S', NULL, options, N_(\"\"),\n+\t\tOPT_CALLBACK_F('S', \"patch-modifies\", options, N_(\"\"),\n \t\t\t N_(\"look for differences that change the number of occurrences of the specified string\"),\n \t\t\t 0, diff_opt_pickaxe_string),\n-\t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n-\t\t\t N_(\"look for differences that change the number of occurrences of the specified regex\"),\n+\t\tOPT_CALLBACK_F('G', \"patch-grep\", options, N_(\"\"),\n+\t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\n-\t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n+\t\t\t N_(\"show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep\"),\n \t\t\t DIFF_PICKAXE_ALL, PARSE_OPT_NONEG),\n \t\tOPT_BIT_F(0, \"pickaxe-regex\", &options->pickaxe_opts,\n-\t\t\t N_(\"treat in -S as extended POSIX regular expression\"),\n+\t\t\t N_(\"treat in -S/--patch-modifies as extended POSIX regular expression\"),\n \t\t\t DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n \t\t\t N_(\"control the order in which files appear in the output\")),\ndiff --git a/diff.h b/diff.h\nindex 6e6007..247ac 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -598,9 +598,16 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n-\" -S find filepair whose only one side contains the string.\\n\" \\\n+\" -G\\n\" \\\n+\" --patch-grep=\\n\" \\\n+\" find differences whose patch contains the regex.\\n\" \\\n+\" -S\\n\" \\\n+\" --patch-modifies=\\n\" \\\n+\" find filepair who differ in the number of occurrences of string.\\n\" \\\n+\" --pickaxe-grep\\n\" \\\n+\" treat as regex in the -S/--patch-modifies argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\n-\" show all files diff when -S is used and hit is found.\\n\" \\\n+\" show all files diff for -G/--patch-grep and -S/--patch-modifies.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n \n int diff_queue_is_empty(struct diff_options *o);\ndiff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh\nindex 8ad3d7..805e0f 100755\n--- a/t/t4062-diff-pickaxe.sh\n+++ b/t/t4062-diff-pickaxe.sh\n@@ -16,13 +16,13 @@ test_expect_success setup '\n '\n \n # OpenBSD only supports up to 255 repetitions, so repeat twice for 64*64=4096.\n-test_expect_success '-G matches' '\n-\tgit diff --name-only -G \"^(0{64}){64}$\" HEAD^ >out &&\n+test_expect_success '--patch-grep matches' '\n+\tgit diff --name-only --patch-grep \"^(0{64}){64}$\" HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \n-test_expect_success '-S --pickaxe-regex' '\n-\tgit diff --name-only -S0 --pickaxe-regex HEAD^ >out &&\n+test_expect_success '--patch-modifies --pickaxe-regex' '\n+\tgit diff --name-only --patch-modifies 0 --pickaxe-regex HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex a675ac..5f4d6 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='log --grep/--author/--regexp-ignore-case/-S/-G'\n+test_description='log --grep/--author/--regexp-ignore-case/--patch-{modifies,grep}'\n \n . ./test-lib.sh\n \n@@ -60,24 +60,48 @@ test_expect_success 'usage' '\n \ttest_expect_code 129 git log -S 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-modifies 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 129 git log -G 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-grep 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 128 git log -Gregex -Sstring 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log -Gregex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex -Sstring 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-grep regex --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-modifies string --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n test_expect_success 'usage: --pickaxe-regex' '\n \ttest_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --pickaxe-regex 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n@@ -89,7 +113,13 @@ test_expect_success 'usage: --no-pickaxe-regex' '\n \ttest_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&\n \ttest_cmp expect actual &&\n \n-\ttest_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&\n+\ttest_expect_code 128 git log --patch-modifies string --no-pickaxe-regex 2>actual &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --no-pickaxe-regex 2>err &&\n \ttest_cmp expect actual\n '\n \n@@ -104,47 +134,59 @@ test_log_icase\texpect_second\t--author person\n test_log_icase\texpect_nomatch\t--author spreon\n \n test_log\texpect_nomatch\t-G picked\n+test_log\texpect_nomatch\t--patch-grep picked\n test_log\texpect_second\t-G Picked\n+test_log\texpect_second\t--patch-grep Picked\n test_log_icase\texpect_nomatch\t-G pickle\n+test_log_icase\texpect_nomatch\t--patch-grep pickle\n test_log_icase\texpect_second\t-G picked\n+test_log_icase\texpect_second\t--patch-grep picked\n \n-test_expect_success 'log -G --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Gfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-grep foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-grep foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n test_log\texpect_nomatch\t-S picked\n+test_log\texpect_nomatch\t--patch-modifies picked\n test_log\texpect_second\t-S Picked\n+test_log\texpect_second\t--patch-modifies Picked\n test_log_icase\texpect_second\t-S picked\n+test_log_icase\texpect_second\t--patch-modifies picked\n test_log_icase\texpect_nomatch\t-S pickle\n+test_log_icase\texpect_nomatch\t--patch-modifies pickle\n \n test_log\texpect_nomatch\t-S p.cked --pickaxe-regex\n+test_log\texpect_nomatch\t--patch-modifies p.cked --pickaxe-regex\n test_log\texpect_second\t-S P.cked --pickaxe-regex\n+test_log\texpect_second\t--patch-modifies P.cked --pickaxe-regex\n test_log_icase\texpect_second\t-S p.cked --pickaxe-regex\n+test_log_icase\texpect_second\t--patch-modifies p.cked --pickaxe-regex\n test_log_icase\texpect_nomatch\t-S p.ckle --pickaxe-regex\n+test_log_icase\texpect_nomatch\t--patch-modifies p.ckle --pickaxe-regex\n \n-test_expect_success 'log -S --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Sfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-modifies foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -S --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-modifies foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n-test_expect_success 'setup log -[GS] plain & regex' '\n+test_expect_success 'setup log --patch{-modifies,-grep} plain & regex' '\n \ttest_create_repo GS-plain &&\n \ttest_commit -C GS-plain --append A data.txt \"a\" &&\n \ttest_commit -C GS-plain --append B data.txt \"a a\" &&\n@@ -159,31 +201,31 @@ test_expect_success 'setup log -[GS] plain & regex' '\n \tgit -C GS-plain log >full-log\n '\n \n-test_expect_success 'log -G trims diff new/old [-+]' '\n-\tgit -C GS-plain log -G\"[+-]a\" >log &&\n+test_expect_success 'log --patch-grep trims diff new/old [-+]' '\n+\tgit -C GS-plain log --patch-grep \"[+-]a\" >log &&\n \ttest_must_be_empty log &&\n-\tgit -C GS-plain log -G\"^a\" >log &&\n+\tgit -C GS-plain log --patch-grep \"^a\" >log &&\n \ttest_cmp log A-to-B-then-E-log\n '\n \n-test_expect_success 'log -S is not a regex, but -S --pickaxe-regex is' '\n-\tgit -C GS-plain log -S\"a\" >log &&\n+test_expect_success 'log --patch-modifies is not a regex, but --patch-modifies --pickaxe-regex is' '\n+\tgit -C GS-plain log --patch-modifies \"a\" >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[a]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" >log &&\n \ttest_must_be_empty log &&\n \n-\tgit -C GS-plain log -S\"[a]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" --pickaxe-regex >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" >log &&\n \ttest_cmp log D-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" --pickaxe-regex >log &&\n \ttest_cmp log C-to-D-then-E-log\n '\n \n-test_expect_success 'setup log -[GS] binary & --text' '\n+test_expect_success 'setup log --patch{-modifies,-grep} binary & --text' '\n \ttest_create_repo GS-bin-txt &&\n \ttest_commit -C GS-bin-txt --printf A data.bin \"a\\na\\0a\\n\" &&\n \ttest_commit -C GS-bin-txt --append --printf B data.bin \"a\\na\\0a\\n\" &&\n@@ -191,36 +233,36 @@ test_expect_success 'setup log -[GS] binary & --text' '\n \tgit -C GS-bin-txt log >full-log\n '\n \n-test_expect_success 'log -G ignores binary files' '\n-\tgit -C GS-bin-txt log -Ga >log &&\n+test_expect_success 'log --patch-grep ignores binary files' '\n+\tgit -C GS-bin-txt log --patch-grep a >log &&\n \ttest_must_be_empty log\n '\n \n-test_expect_success 'log -G looks into binary files with -a' '\n-\tgit -C GS-bin-txt log -a -Ga >log &&\n+test_expect_success 'log --patch-grep looks into binary files with -a' '\n+\tgit -C GS-bin-txt log -a --patch-grep a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -G looks into binary files with textconv filter' '\n+test_expect_success 'log --patch-grep looks into binary files with textconv filter' '\n \ttest_when_finished \"rm GS-bin-txt/.gitattributes\" &&\n \t(\n \t\tcd GS-bin-txt &&\n \t\techo \"* diff=bin\" >.gitattributes &&\n-\t\tgit -c diff.bin.textconv=cat log -Ga >../log\n+\t\tgit -c diff.bin.textconv=cat log --patch-grep a >../log\n \t) &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S looks into binary files' '\n-\tgit -C GS-bin-txt log -Sa >log &&\n+test_expect_success 'log --patch-modifies looks into binary files' '\n+\tgit -C GS-bin-txt log --patch-modifies a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S --pickaxe-regex looks into binary files' '\n-\tgit -C GS-bin-txt log --pickaxe-regex -Sa >log &&\n+test_expect_success 'log --patch-modifies --pickaxe-regex looks into binary files' '\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies a >log &&\n \ttest_cmp log full-log &&\n \n-\tgit -C GS-bin-txt log --pickaxe-regex -S\"[a]\" >log &&\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies \"[a]\" >log &&\n \ttest_cmp log full-log\n '\n \n-- \n2.45.2\n\n"},{"id":"511996","messageId":"xmqq4j17kyvk.fsf@gitster.g","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"Re: [PATCH v3 0/1] Long names for `git log -S` and `git log -G`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-06T13:04:47Z","receivedAt":"2025-02-06T13:04:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> Same as PATCH v2[1], but removed gitk changes, as suggested by Johannes Sixt.\n>\n> I'll send a separate patch for gitk, should this patch be accepted.\n> Or, I could include gitk changes into this chain, but just as a separate patch?\n\nThey are technically separate codebases so the changes to the core\nwould become prerequisite for the same changes to gitk, so from that\npoint of view the former may be more kosher, but I would expect that\nthe latter is fine in practice.\n"},{"id":"512016","messageId":"xmqqseoqiybi.fsf@gitster.g","threadId":"62909","inReplyTo":"20250206014324.1839232-2-illia.bobyr@gmail.com","subject":"Re: [PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-06T20:59:45Z","receivedAt":"2025-02-06T20:59:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> Most arguments have both short and long versions. Long versions are\n> easier to read, especially in scripts and command history.\n>\n> Tests that check just the option parsing are duplicated to check both\n> short and long argument options. But more complex tests are updated to\n> use the long argument in order to improve the test readability.\n\nWhile checking both may be a prudent thing to do, because the \"-S\"\noption and the \"-G\" option have been there with us almost since the\nbeginning of time, the swapping all existing use of them with the\nlonghand is rather unwelcome and needless churn, I would have to\nsay.\n\n> `-S`::\n> +`--patch-modifies=`::\n\nGood. I am looking at 'git-commit.txt' as an example, and we seem\nto give shorthand first and then longhand, which matches what we see\nhere.\n\n> @@ -657,18 +658,19 @@ renamed entries cannot appear if detection for those types is disabled.\n> It is useful when you're looking for an exact block of code (like a\n> struct), and want to know the history of that block since it first\n> came into being: use the feature iteratively to feed the interesting\n> -block in the preimage back into `-S`, and keep going until you get the\n> -very first version of the block.\n> +block in the preimage back into `--patch-modifies`, and keep going until\n> +you get the very first version of the block.\n\nIf this paragraph _were_ written with the longhand from the\nbeginning, I would not have minded too much, but I personally find\nit unnecessary to churn the existing document like this.\n\n> `-G`::\n> +`--patch-grep=`::\n\nSame two paragraphs from the above apply here, and ...\n\n> `--find-object=`::\n> `--pickaxe-all`::\n> `--pickaxe-regex`::\n\n... all of the above.\n\n> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\n\nDitto.\n\n> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\n> index 642c5..e4b18 100644\n> --- a/Documentation/gitdiffcore.txt\n> +++ b/Documentation/gitdiffcore.txt\n> @@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n> \n> This transformation limits the set of filepairs to those that change\n> specified strings between the preimage and the postimage in a certain\n> -way. -S and -G options are used to\n> +way. --patch-modifies= and\n> +--patch-grep= options are used to specify\n> +different ways these strings are sought.\n\nThis is worse. Here is the first part that describes the pickaxe,\nso mentioning both may be more appropriate; showing only the\nlonghand nobody is familiar with (yet) does not make any sense.\n\n ... certain way. `--patch-modifies=`\n (`-S` for short) and `--patch-grep=`\n (`-G` for short) are used to ...\n\nOnce establishing the equivalence between the longhand and the\nshorthand for these two options, we do not have to churn the\nexisting text at all.\n\n> diff --git a/diff.c b/diff.c\n> index d28b41..09beb 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)\n> \n> \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n> \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n> -\t\t\t\"-G\", \"-S\", \"--find-object\");\n> +\t\t\t\"-G/--patch-grep\", \"-S/--patch-modifies\", \"--find-object\");\n> \n> \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))\n> \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s'\"),\n> -\t\t\t\"-G\", \"--pickaxe-regex\", \"--pickaxe-regex\", \"-S\");\n> +\t\t\t\"-G/--patch-grep\", \"--pickaxe-regex\",\n> + \"--pickaxe-regex\", \"-S/--patch-modifies\");\n> \n> \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))\n> \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'\"),\n> -\t\t\t\"--pickaxe-all\", \"--find-object\", \"--pickaxe-all\", \"-G\", \"-S\");\n> +\t\t\t\"--pickaxe-all\", \"--find-object\",\n> + \"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\n\nThe message change looks fine; the indentation is broken.\n\n.git/rebase-apply/patch:184: indent with spaces.\n \"--pickaxe-regex\", \"-S/--patch-modifies\");\n.git/rebase-apply/patch:190: indent with spaces.\n \"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\nwarning: 2 lines applied after fixing whitespace errors.\nApplying: diff: --patch-{modifies,grep} arg names for -S and -G\n\nThese alone do not require a new iteration, as \"git am --whitespace=fix\"\nalready corrected them.\n\n> -\t\tOPT_CALLBACK_F('S', NULL, options, N_(\"\"),\n> +\t\tOPT_CALLBACK_F('S', \"patch-modifies\", options, N_(\"\"),\n> -\t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n> +\t\tOPT_CALLBACK_F('G', \"patch-grep\", options, N_(\"\"),\n\nOK. NOte that this says . We may want to have a separate clean-up\npatch so that Documentation/gitdifcore.txt that used \nand the placeholder used here match.\n\n> -\t\t\t N_(\"look for differences that change the number of occurrences of the specified regex\"),\n> +\t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n\nThis is an unrelated change that should not be in this patch. If\nyou want to modify it, please do it in a separate clean-up patch,\njust like the above vs change.\n\n> -\t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n> +\t\t\t N_(\"show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep\"),\n\nThis line is meant to be shown when the user requests list of\noptions and their meanings. Growing the message from 47 columns or\nso to 78 columns would make it wider than terminals when these\nmessages are indented. Because earlier entries in this array have\nalready established the equivalence between the shorthand and the\nlonghand, I do not think the output is understandable without this\nchange.\n\n> \t\tOPT_BIT_F(0, \"pickaxe-regex\", &options->pickaxe_opts,\n> -\t\t\t N_(\"treat in -S as extended POSIX regular expression\"),\n> +\t\t\t N_(\"treat in -S/--patch-modifies as extended POSIX regular expression\"),\n\nDitto.\n\nThanks.\n"},{"id":"512214","messageId":"20250211085028.3923875-1-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 0/10] Long names for `git log -S` and `git log -G`","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:12Z","receivedAt":"2025-02-11T08:50:44Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"I've split the big change from v3 [1] into multiple, mostly independent patches\nto make it easier to review and merge each one separately.\n\n[1] https://lore.kernel.org/git/20250206014324.1839232-1-illia.bobyr@gmail.com/\n\nPatches 1 through 4 are fixing minor bugs and inconsistencies.\n\nPatch 5 contains updates gitdiffcore to use same placeholder names as the rest\nof the code.\n\nPatch 6 contains a minimum change to add long versions of -S and -G.\n\nPatch 7 adds bash completion support.\n\nPatches 8 through 10 increase usage of the long argument versions in tests, CLI\nhelp and docs respectively.\n\nPlease, let me know if you prefer it split in a different way, or reorder the\nchanges.\n\n---\n\nI was not sure if I should include a reference to the previous version of the\npatch into the next reroll. It seems that\n`Documentation/MyFirstContribution.adoc` suggests so. But it creates very long\nthreads. And I've noticed that not everyone is doing it.\n\n---\n\nReply to review notes from Junio C Hamano:\n\nOn 2/6/25 12:59, Junio C Hamano wrote:\n> Illia Bobyr writes:\n>\n>> Most arguments have both short and long versions. Long versions are\n>> easier to read, especially in scripts and command history.\n>>\n>> Tests that check just the option parsing are duplicated to check both\n>> short and long argument options. But more complex tests are updated to\n>> use the long argument in order to improve the test readability.\n>\n> While checking both may be a prudent thing to do, because the \"-S\"\n> option and the \"-G\" option have been there with us almost since the\n> beginning of time, the swapping all existing use of them with the\n> longhand is rather unwelcome and needless churn, I would have to\n> say.\n\nMy thinking is that as long version names improve readability, it also applies\nto the test code. When I see a short option I often have to check the manual to\nremember what exactly does it do. Even for \"-S\"/\"-G\" I find myself sometimes\nconfused as to which of the two does what exactly. While the \"grep\" mnemonic\nhelps, I do not always remember it.\n\nBut, I think, I understand your point of view as well.\n\nIn v4 patch 5 contains a relatively minimum amount of changes that add long\nalternatives for \"-S\" and \"-G\" just to the command line parsing.\n\nI do not have your experience with assessing the churn, but if my argument about\nthe readability changes your mind, I've moved the rest of the updates into\nseparate patches, at the end of the chain. Patches 8 through 10. Making it\neasier to discuss them in smaller chunks, if you wish so. But also, I assume,\nit should be easy for you to ignore those, if you do not want to include them?\n\n> [...]\n>> @@ -657,18 +658,19 @@ renamed entries cannot appear if detection for those types is disabled.\n>> It is useful when you're looking for an exact block of code (like a\n>> struct), and want to know the history of that block since it first\n>> came into being: use the feature iteratively to feed the interesting\n>> -block in the preimage back into `-S`, and keep going until you get the\n>> -very first version of the block.\n>> +block in the preimage back into `--patch-modifies`, and keep going until\n>> +you get the very first version of the block.\n>\n> If this paragraph _were_ written with the longhand from the\n> beginning, I would not have minded too much, but I personally find\n> it unnecessary to churn the existing document like this.\n>\n>> `-G`::\n>> +`--patch-grep=`::\n>\n> Same two paragraphs from the above apply here, and ...\n>\n>> `--find-object=`::\n>> `--pickaxe-all`::\n>> `--pickaxe-regex`::\n>\n> ... all of the above.\n>\n>> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\n>\n> Ditto.\n>\n>> diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\n>> index 642c5..e4b18 100644\n>> --- a/Documentation/gitdiffcore.txt\n>> +++ b/Documentation/gitdiffcore.txt\n>> @@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n>> \n>> This transformation limits the set of filepairs to those that change\n>> specified strings between the preimage and the postimage in a certain\n>> -way. -S and -G options are used to\n>> +way. --patch-modifies= and\n>> +--patch-grep= options are used to specify\n>> +different ways these strings are sought.\n>\n> This is worse. Here is the first part that describes the pickaxe,\n> so mentioning both may be more appropriate; showing only the\n> longhand nobody is familiar with (yet) does not make any sense.\n>\n> ... certain way. `--patch-modifies=`\n> (`-S` for short) and `--patch-grep=`\n> (`-G` for short) are used to ...\n>\n> Once establishing the equivalence between the longhand and the\n> shorthand for these two options, we do not have to churn the\n> existing text at all.\n\nApplied your suggestion.\n\nI guess one difference in the way you look at it, is that you default to the\nshort version when you can. While I default to the long one, as I assume it is\neasier to understand. Someone not that intimately familiar with git might need\nto go to the previous paragraph to recall what \"-S\" and \"-G\" are, while if they\nare spelled as \"--patch-modifies\" and \"--patch-grep\", it might be less\nnecessary. So, the argument is that while we are reducing the diff, we might\nalso be reducing the improvement in readability.\n\nBeing the author, I could also be biased when assessing how much more readable\n\"--patch-modifies\" and \"--patch-grep\" are compared to \"-S\" and \"-G\".\n\n>> diff --git a/diff.c b/diff.c\n>> index d28b41..09beb 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)\n>> \n>> \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n>> \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n>> -\t\t\t\"-G\", \"-S\", \"--find-object\");\n>> +\t\t\t\"-G/--patch-grep\", \"-S/--patch-modifies\", \"--find-object\");\n>> \n>> \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))\n>> \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s'\"),\n>> -\t\t\t\"-G\", \"--pickaxe-regex\", \"--pickaxe-regex\", \"-S\");\n>> +\t\t\t\"-G/--patch-grep\", \"--pickaxe-regex\",\n>> + \"--pickaxe-regex\", \"-S/--patch-modifies\");\n>> \n>> \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))\n>> \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'\"),\n>> -\t\t\t\"--pickaxe-all\", \"--find-object\", \"--pickaxe-all\", \"-G\", \"-S\");\n>> +\t\t\t\"--pickaxe-all\", \"--find-object\",\n>> + \"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\n>\n> The message change looks fine; the indentation is broken.\n>\n> .git/rebase-apply/patch:184: indent with spaces.\n> \"--pickaxe-regex\", \"-S/--patch-modifies\");\n> .git/rebase-apply/patch:190: indent with spaces.\n> \"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\n> warning: 2 lines applied after fixing whitespace errors.\n> Applying: diff: --patch-{modifies,grep} arg names for -S and -G\n>\n> These alone do not require a new iteration, as \"git am --whitespace=fix\"\n> already corrected them.\n\nSorry about this. I did check the indentation manually, but did not use a\ntool. Reconfigured my editor to use tabs now.\n\n>> -\t\tOPT_CALLBACK_F('S', NULL, options, N_(\"\"),\n>> +\t\tOPT_CALLBACK_F('S', \"patch-modifies\", options, N_(\"\"),\n>> -\t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n>> +\t\tOPT_CALLBACK_F('G', \"patch-grep\", options, N_(\"\"),\n>\n> OK. NOte that this says . We may want to have a separate clean-up\n> patch so that Documentation/gitdifcore.txt that used \n> and the placeholder used here match.\n\nMakes sense.\nI've added this fix as patch 5.\nI've reformatted paragraphs in gitdifcore.adoc that were affected.\nLet me know if you do not want me to reformat it, and just keep shorter lines as\nis.\n\n>> -\t\t\t N_(\"look for differences that change the number of occurrences of the specified regex\"),\n>> +\t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n>\n> This is an unrelated change that should not be in this patch. If\n> you want to modify it, please do it in a separate clean-up patch,\n> just like the above vs change.\n\nSplit it into patch 2.\n\n>> -\t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n>> +\t\t\t N_(\"show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep\"),\n>\n> This line is meant to be shown when the user requests list of\n> options and their meanings. Growing the message from 47 columns or\n> so to 78 columns would make it wider than terminals when these\n> messages are indented. Because earlier entries in this array have\n> already established the equivalence between the shorthand and the\n> longhand, I do not think the output is understandable without this\n> change.\n\nA description for \"-S\" is already 81 characters long:\n\nN_(\"look for differences that change the number of occurrences of the specified regex\"),\n\nSo I was assuming if I grow another description to 77 characters it is still OK.\nWhile one can find the correspondence between \"-S\" and \"--patch-modifies\" by\nreading the \"-S\" description, in my mind, the same argument applies as to the\ntest readability - it just makes it a bit easier for the reader.\n\nThis change is now part of a much smaller patch 9, which is only about adding\nlonger alternatives to the CLI help messages that currently contain only \"-G\"\nand \"-S\". This way you can decide if you want it or not as a complete unit. Or\nif you want me to change it in some way, we can discuss it separately from the\nrest of the changes.\n\nBy the way, I must admit I can not find a way to look at a help message\ngenerated from these strings. Running `git diff -h` shows a message from\n`diff.h` and running `git diff --help` shows the man page.\n\n---\n\nIllia Bobyr (10):\n t/t4209-log-pickaxe: Naming typo: -G takes a regex\n diff: -G description: Correct copy/paste error\n diff: short help: Correct -S description\n diff: short help: Add -G and --pickaxe-grep\n docs: gitdiffcore: -G and -S: Use regex/string placeholders\n diff: --patch-{grep,modifies} arg names for -G and -S\n completion: Support --patch-{grep,modifies}\n diff: test: Use --patch-{grep,modifies} over -G/-S\n diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}\n diff: docs: Use --patch-{grep,modifies} over -G/-S\n\n Documentation/diff-options.adoc | 36 +++++----\n Documentation/git-blame.adoc | 2 +-\n Documentation/gitdiffcore.adoc | 55 ++++++-------\n contrib/completion/git-completion.bash | 11 ++-\n diff.c | 18 +++--\n diff.h | 11 ++-\n t/t4062-diff-pickaxe.sh | 8 +-\n t/t4209-log-pickaxe.sh | 106 +++++++++++++++++--------\n 8 files changed, 155 insertions(+), 92 deletions(-)\n\n-- \n2.45.2\n\n"},{"id":"512215","messageId":"20250211085028.3923875-2-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:13Z","receivedAt":"2025-02-11T08:50:51Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Not effect on the test logic, but as \"-G\" argument is a regex it is more\naccurate to use \"regex\" as a dummy argument value rather than \"string\".\nIn all the other case when \"-G\" is passed a dummy value it is spelled as\n\"regex\" rather than as \"string\".\n---\n t/t4209-log-pickaxe.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex a675ac..ed70c 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -89,7 +89,7 @@ test_expect_success 'usage: --no-pickaxe-regex' '\n \ttest_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&\n \ttest_cmp expect actual &&\n \n-\ttest_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&\n+\ttest_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&\n \ttest_cmp expect actual\n '\n \n-- \n2.45.2\n\n"},{"id":"512216","messageId":"20250211085028.3923875-3-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 02/10] diff: -G description: Correct copy/paste error","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:14Z","receivedAt":"2025-02-11T08:50:53Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Current description for -G is incorrect, seems like it was copied from\nthe description for -S.\n---\n diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 019fb..bd9db 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5866,7 +5866,7 @@ struct option *add_diff_options(const struct option *opts,\n \t\t\t N_(\"look for differences that change the number of occurrences of the specified string\"),\n \t\t\t 0, diff_opt_pickaxe_string),\n \t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n-\t\t\t N_(\"look for differences that change the number of occurrences of the specified regex\"),\n+\t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\n \t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n-- \n2.45.2\n\n"},{"id":"512217","messageId":"20250211085028.3923875-4-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 03/10] diff: short help: Correct -S description","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:15Z","receivedAt":"2025-02-11T08:50:56Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"`-S` shows changes that modify the number of occurrences of the\nspecified string, rather than only those that either completely remove\nit or add it for the first time.\n---\n diff.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.h b/diff.h\nindex 0a566f..49ece3 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -606,7 +606,7 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n-\" -S find filepair whose only one side contains the string.\\n\" \\\n+\" -S find filepair who differ in the number of occurrences of string.\\n\" \\\n \" --pickaxe-all\\n\" \\\n \" show all files diff when -S is used and hit is found.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n-- \n2.45.2\n\n"},{"id":"512218","messageId":"20250211085028.3923875-5-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 04/10] diff: short help: Add -G and --pickaxe-grep","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:16Z","receivedAt":"2025-02-11T08:50:58Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"-G and --pickaxe-grep seems to be on par with -S and --pickaxe-all that\nare already mentioned.\n---\n diff.h | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.h b/diff.h\nindex 49ece3..787bb 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -606,9 +606,12 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n+\" -G find differences where patch contains the specified regex.\\n\" \\\n \" -S find filepair who differ in the number of occurrences of string.\\n\" \\\n+\" --pickaxe-grep\\n\" \\\n+\" treat as a regex in the -S argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\n-\" show all files diff when -S is used and hit is found.\\n\" \\\n+\" show all files diff when -G or -S is used and hit is found.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n \n int diff_queue_is_empty(struct diff_options *o);\n-- \n2.45.2\n\n"},{"id":"512219","messageId":"20250211085028.3923875-6-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 05/10] docs: gitdiffcore: -G and -S: Use regex/string placeholders","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:17Z","receivedAt":"2025-02-11T08:51:00Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"In the rest of the documentation (and in the code) we use `regex` and\n`string` as `-G` and `-S` argument placeholders. While\n`regular-expression` and `block-of-text` are a bit easier to read, it is\na bit consistent.\n\nAnd we could assume that everyone who uses git should be able to\nunderstand that a \"string\" and a \"block-of-text\", as well as a \"regex\"\nand \"regular-expression\" are the same thing. So, using a shorter\nversion is also more consistent.\n---\n Documentation/gitdiffcore.adoc | 23 +++++++++++------------\n 1 file changed, 11 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/gitdiffcore.adoc b/Documentation/gitdiffcore.adoc\nindex 642c5..0d7d66 100644\n--- a/Documentation/gitdiffcore.adoc\n+++ b/Documentation/gitdiffcore.adoc\n@@ -245,26 +245,25 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. -S and -G options are used to\n-specify different ways these strings are sought.\n+way. `-S` and `-G` options are used to specify\n+different ways these strings are sought.\n \n-\"-S\" detects filepairs whose preimage and postimage\n-have different number of occurrences of the specified block of text.\n+`-S` detects filepairs whose preimage and postimage\n+have different number of occurrences of the specified __.\n By definition, it will not detect in-file moves. Also, when a\n changeset moves a file wholesale without affecting the interesting\n string, diffcore-rename kicks in as usual, and `-S` omits the filepair\n (since the number of occurrences of that string didn't change in that\n rename-detected filepair). When used with `--pickaxe-regex`, treat\n-the as an extended POSIX regular expression to match,\n+the __ as an extended POSIX regular expression to match,\n instead of a literal string.\n \n-\"-G\" (mnemonic: grep) detects filepairs whose\n-textual diff has an added or a deleted line that matches the given\n-regular expression. This means that it will detect in-file (or what\n-rename-detection considers the same file) moves, which is noise. The\n-implementation runs diff twice and greps, and this can be quite\n-expensive. To speed things up, binary files without textconv filters\n-will be ignored.\n+`-G` (mnemonic: grep) detects filepairs whose textual diff has\n+an added or a deleted line that matches the given __. This\n+means that it will detect in-file (or what rename-detection considers\n+the same file) moves, which is noise. The implementation runs diff\n+twice and greps, and this can be quite expensive. To speed things up,\n+binary files without textconv filters will be ignored.\n \n When `-S` or `-G` are used without `--pickaxe-all`, only filepairs\n that match their respective criterion are kept in the output. When\n-- \n2.45.2\n\n"},{"id":"512220","messageId":"20250211085028.3923875-7-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 06/10] diff: --patch-{grep,modifies} arg names for -G and -S","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:18Z","receivedAt":"2025-02-11T08:51:01Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Most arguments have both short and long versions. Long versions are\neasier to read, especially in scripts and command history.\n\nThis change mostly keeps existing uses of -G and -S as is in the tests,\ndocumentation and help output.\n\nTests that check just the option parsing are duplicated to check both\nshort and long argument options.\n\nSigned-off-by: Illia Bobyr \n---\n Documentation/diff-options.adoc | 2 ++\n Documentation/gitdiffcore.adoc | 3 ++-\n diff.c | 12 ++++++----\n diff.h | 8 +++++--\n t/t4209-log-pickaxe.sh | 42 +++++++++++++++++++++++++++++++++\n 5 files changed, 59 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex 640eb..07413d 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -650,6 +650,7 @@ Note that not all diffs can feature all types. For instance, copied and\n renamed entries cannot appear if detection for those types is disabled.\n \n `-S`::\n+`--patch-modifies=`::\n \tLook for differences that change the number of occurrences of\n \tthe specified __ (i.e. addition/deletion) in a file.\n \tIntended for the scripter's use.\n@@ -663,6 +664,7 @@ very first version of the block.\n Binary files are searched as well.\n \n `-G`::\n+`--patch-grep=`::\n \tLook for differences whose patch text contains added/removed\n \tlines that match __.\n +\ndiff --git a/Documentation/gitdiffcore.adoc b/Documentation/gitdiffcore.adoc\nindex 0d7d66..e934b9 100644\n--- a/Documentation/gitdiffcore.adoc\n+++ b/Documentation/gitdiffcore.adoc\n@@ -245,7 +245,8 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. `-S` and `-G` options are used to specify\n+way. `--patch-modifies=` (`-S` for short) and\n+`--patch-grep=` (`-G` for short) are used to specify\n different ways these strings are sought.\n \n `-S` detects filepairs whose preimage and postimage\ndiff --git a/diff.c b/diff.c\nindex bd9db..ac2cd 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n-\t\t\t\"-G\", \"-S\", \"--find-object\");\n+\t\t\t\"-G/--patch-grep\", \"-S/--patch-modifies\", \"--find-object\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s'\"),\n-\t\t\t\"-G\", \"--pickaxe-regex\", \"--pickaxe-regex\", \"-S\");\n+\t\t\t\"-G/--patch-grep\", \"--pickaxe-regex\",\n+\t\t\t\"--pickaxe-regex\", \"-S/--patch-modifies\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'\"),\n-\t\t\t\"--pickaxe-all\", \"--find-object\", \"--pickaxe-all\", \"-G\", \"-S\");\n+\t\t\t\"--pickaxe-all\", \"--find-object\",\n+\t\t\t\"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\n \n \t/*\n \t * Most of the time we can say \"there are changes\"\n@@ -5862,10 +5864,10 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_SET_INT_F(0, \"ita-visible-in-index\", &options->ita_invisible_in_index,\n \t\t\t N_(\"treat 'git add -N' entries as real in the index\"),\n \t\t\t 0, PARSE_OPT_NONEG),\n-\t\tOPT_CALLBACK_F('S', NULL, options, N_(\"\"),\n+\t\tOPT_CALLBACK_F('S', \"patch-modifies\", options, N_(\"\"),\n \t\t\t N_(\"look for differences that change the number of occurrences of the specified string\"),\n \t\t\t 0, diff_opt_pickaxe_string),\n-\t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n+\t\tOPT_CALLBACK_F('G', \"patch-grep\", options, N_(\"\"),\n \t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\ndiff --git a/diff.h b/diff.h\nindex 787bb..ed48a 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -606,8 +606,12 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n-\" -G find differences where patch contains the specified regex.\\n\" \\\n-\" -S find filepair who differ in the number of occurrences of string.\\n\" \\\n+\" -G\\n\" \\\n+\" --patch-grep=\\n\" \\\n+\" find differences where patch contains the regex.\\n\" \\\n+\" -S\\n\" \\\n+\" --patch-modifies=\\n\" \\\n+\" find filepair who differ in the number of occurrences of string.\\n\" \\\n \" --pickaxe-grep\\n\" \\\n \" treat as a regex in the -S argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex ed70c..ab14b 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -60,24 +60,48 @@ test_expect_success 'usage' '\n \ttest_expect_code 129 git log -S 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-modifies 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 129 git log -G 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-grep 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 128 git log -Gregex -Sstring 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log -Gregex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex -Sstring 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-grep regex --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-modifies string --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n test_expect_success 'usage: --pickaxe-regex' '\n \ttest_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --pickaxe-regex 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n@@ -89,7 +113,13 @@ test_expect_success 'usage: --no-pickaxe-regex' '\n \ttest_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&\n \ttest_cmp expect actual &&\n \n+\ttest_expect_code 128 git log --patch-modifies string --no-pickaxe-regex 2>actual &&\n+\ttest_cmp expect actual &&\n+\n \ttest_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --no-pickaxe-regex 2>err &&\n \ttest_cmp expect actual\n '\n \n@@ -104,9 +134,13 @@ test_log_icase\texpect_second\t--author person\n test_log_icase\texpect_nomatch\t--author spreon\n \n test_log\texpect_nomatch\t-G picked\n+test_log\texpect_nomatch\t--patch-grep picked\n test_log\texpect_second\t-G Picked\n+test_log\texpect_second\t--patch-grep Picked\n test_log_icase\texpect_nomatch\t-G pickle\n+test_log_icase\texpect_nomatch\t--patch-grep pickle\n test_log_icase\texpect_second\t-G picked\n+test_log_icase\texpect_second\t--patch-grep picked\n \n test_expect_success 'log -G --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n@@ -122,14 +156,22 @@ test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n '\n \n test_log\texpect_nomatch\t-S picked\n+test_log\texpect_nomatch\t--patch-modifies picked\n test_log\texpect_second\t-S Picked\n+test_log\texpect_second\t--patch-modifies Picked\n test_log_icase\texpect_second\t-S picked\n+test_log_icase\texpect_second\t--patch-modifies picked\n test_log_icase\texpect_nomatch\t-S pickle\n+test_log_icase\texpect_nomatch\t--patch-modifies pickle\n \n test_log\texpect_nomatch\t-S p.cked --pickaxe-regex\n+test_log\texpect_nomatch\t--patch-modifies p.cked --pickaxe-regex\n test_log\texpect_second\t-S P.cked --pickaxe-regex\n+test_log\texpect_second\t--patch-modifies P.cked --pickaxe-regex\n test_log_icase\texpect_second\t-S p.cked --pickaxe-regex\n+test_log_icase\texpect_second\t--patch-modifies p.cked --pickaxe-regex\n test_log_icase\texpect_nomatch\t-S p.ckle --pickaxe-regex\n+test_log_icase\texpect_nomatch\t--patch-modifies p.ckle --pickaxe-regex\n \n test_expect_success 'log -S --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-- \n2.45.2\n\n"},{"id":"512221","messageId":"20250211085028.3923875-8-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 07/10] completion: Support --patch-{grep,modifies}","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:19Z","receivedAt":"2025-02-11T08:51:01Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"---\n contrib/completion/git-completion.bash | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 41391..daf335 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1900,6 +1900,7 @@ __git_diff_common_options=\"--stat --numstat --shortstat --summary\n \t\t\t--output= --output-indicator-context=\n \t\t\t--output-indicator-new= --output-indicator-old=\n \t\t\t--ws-error-highlight=\n+\t\t\t--patch-grep= --patch-modifies=\n \t\t\t--pickaxe-all --pickaxe-regex --patch-with-raw\n \"\n \n@@ -2216,7 +2217,7 @@ __git_complete_log_opts ()\n \t\t__git_complete_symbol --cur=\"${cur#:}\" --sfx=\":\"\n \t\treturn\n \t\t;;\n-\t-G,*|-S,*)\n+\t-G,*|--patch-grep,*|-S,*|--patch-modifies,*)\n \t\t__git_complete_symbol\n \t\treturn\n \t\t;;\n@@ -2239,6 +2240,14 @@ __git_complete_log_opts ()\n \t\t__gitcomp \"$__git_diff_algorithms\" \"\" \"${cur##--diff-algorithm=}\"\n \t\treturn\n \t\t;;\n+\t--patch-grep=*)\n+\t\t__git_complete_symbol --pfx=\"--patch-grep=\" --cur=\"${cur#--patch-grep=}\"\n+\t\treturn\n+\t\t;;\n+\t--patch-modifies=*)\n+\t\t__git_complete_symbol --pfx=\"--patch-modifies=\" --cur=\"${cur#--patch-modifies=}\"\n+\t\treturn\n+\t\t;;\n \t--submodule=*)\n \t\t__gitcomp \"$__git_diff_submodule_formats\" \"\" \"${cur##--submodule=}\"\n \t\treturn\n-- \n2.45.2\n\n"},{"id":"512222","messageId":"20250211085028.3923875-9-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 08/10] diff: test: Use --patch-{grep,modifies} over -G/-S","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:20Z","receivedAt":"2025-02-11T08:51:02Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Long argument names are easier to read, compared to short ones. So\nwhile short arguments are great when you want to type a command quickly,\ntests are more readable if we use long argument names.\n\nThere are still test that verify that both short and long arguments work\ninterchangeably when parsing the arguments.\n\nTests where the focus is not on the argument names are updated to use\nlong argument names.\n---\n t/t4062-diff-pickaxe.sh | 8 +++---\n t/t4209-log-pickaxe.sh | 62 ++++++++++++++++++++---------------------\n 2 files changed, 35 insertions(+), 35 deletions(-)\n\ndiff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh\nindex 8ad3d7..805e0f 100755\n--- a/t/t4062-diff-pickaxe.sh\n+++ b/t/t4062-diff-pickaxe.sh\n@@ -16,13 +16,13 @@ test_expect_success setup '\n '\n \n # OpenBSD only supports up to 255 repetitions, so repeat twice for 64*64=4096.\n-test_expect_success '-G matches' '\n-\tgit diff --name-only -G \"^(0{64}){64}$\" HEAD^ >out &&\n+test_expect_success '--patch-grep matches' '\n+\tgit diff --name-only --patch-grep \"^(0{64}){64}$\" HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \n-test_expect_success '-S --pickaxe-regex' '\n-\tgit diff --name-only -S0 --pickaxe-regex HEAD^ >out &&\n+test_expect_success '--patch-modifies --pickaxe-regex' '\n+\tgit diff --name-only --patch-modifies 0 --pickaxe-regex HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex ab14b..5f4d6 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='log --grep/--author/--regexp-ignore-case/-S/-G'\n+test_description='log --grep/--author/--regexp-ignore-case/--patch-{modifies,grep}'\n \n . ./test-lib.sh\n \n@@ -142,15 +142,15 @@ test_log_icase\texpect_nomatch\t--patch-grep pickle\n test_log_icase\texpect_second\t-G picked\n test_log_icase\texpect_second\t--patch-grep picked\n \n-test_expect_success 'log -G --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Gfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-grep foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-grep foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n@@ -173,20 +173,20 @@ test_log_icase\texpect_second\t--patch-modifies p.cked --pickaxe-regex\n test_log_icase\texpect_nomatch\t-S p.ckle --pickaxe-regex\n test_log_icase\texpect_nomatch\t--patch-modifies p.ckle --pickaxe-regex\n \n-test_expect_success 'log -S --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Sfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-modifies foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -S --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-modifies foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n-test_expect_success 'setup log -[GS] plain & regex' '\n+test_expect_success 'setup log --patch{-modifies,-grep} plain & regex' '\n \ttest_create_repo GS-plain &&\n \ttest_commit -C GS-plain --append A data.txt \"a\" &&\n \ttest_commit -C GS-plain --append B data.txt \"a a\" &&\n@@ -201,31 +201,31 @@ test_expect_success 'setup log -[GS] plain & regex' '\n \tgit -C GS-plain log >full-log\n '\n \n-test_expect_success 'log -G trims diff new/old [-+]' '\n-\tgit -C GS-plain log -G\"[+-]a\" >log &&\n+test_expect_success 'log --patch-grep trims diff new/old [-+]' '\n+\tgit -C GS-plain log --patch-grep \"[+-]a\" >log &&\n \ttest_must_be_empty log &&\n-\tgit -C GS-plain log -G\"^a\" >log &&\n+\tgit -C GS-plain log --patch-grep \"^a\" >log &&\n \ttest_cmp log A-to-B-then-E-log\n '\n \n-test_expect_success 'log -S is not a regex, but -S --pickaxe-regex is' '\n-\tgit -C GS-plain log -S\"a\" >log &&\n+test_expect_success 'log --patch-modifies is not a regex, but --patch-modifies --pickaxe-regex is' '\n+\tgit -C GS-plain log --patch-modifies \"a\" >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[a]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" >log &&\n \ttest_must_be_empty log &&\n \n-\tgit -C GS-plain log -S\"[a]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" --pickaxe-regex >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" >log &&\n \ttest_cmp log D-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" --pickaxe-regex >log &&\n \ttest_cmp log C-to-D-then-E-log\n '\n \n-test_expect_success 'setup log -[GS] binary & --text' '\n+test_expect_success 'setup log --patch{-modifies,-grep} binary & --text' '\n \ttest_create_repo GS-bin-txt &&\n \ttest_commit -C GS-bin-txt --printf A data.bin \"a\\na\\0a\\n\" &&\n \ttest_commit -C GS-bin-txt --append --printf B data.bin \"a\\na\\0a\\n\" &&\n@@ -233,36 +233,36 @@ test_expect_success 'setup log -[GS] binary & --text' '\n \tgit -C GS-bin-txt log >full-log\n '\n \n-test_expect_success 'log -G ignores binary files' '\n-\tgit -C GS-bin-txt log -Ga >log &&\n+test_expect_success 'log --patch-grep ignores binary files' '\n+\tgit -C GS-bin-txt log --patch-grep a >log &&\n \ttest_must_be_empty log\n '\n \n-test_expect_success 'log -G looks into binary files with -a' '\n-\tgit -C GS-bin-txt log -a -Ga >log &&\n+test_expect_success 'log --patch-grep looks into binary files with -a' '\n+\tgit -C GS-bin-txt log -a --patch-grep a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -G looks into binary files with textconv filter' '\n+test_expect_success 'log --patch-grep looks into binary files with textconv filter' '\n \ttest_when_finished \"rm GS-bin-txt/.gitattributes\" &&\n \t(\n \t\tcd GS-bin-txt &&\n \t\techo \"* diff=bin\" >.gitattributes &&\n-\t\tgit -c diff.bin.textconv=cat log -Ga >../log\n+\t\tgit -c diff.bin.textconv=cat log --patch-grep a >../log\n \t) &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S looks into binary files' '\n-\tgit -C GS-bin-txt log -Sa >log &&\n+test_expect_success 'log --patch-modifies looks into binary files' '\n+\tgit -C GS-bin-txt log --patch-modifies a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S --pickaxe-regex looks into binary files' '\n-\tgit -C GS-bin-txt log --pickaxe-regex -Sa >log &&\n+test_expect_success 'log --patch-modifies --pickaxe-regex looks into binary files' '\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies a >log &&\n \ttest_cmp log full-log &&\n \n-\tgit -C GS-bin-txt log --pickaxe-regex -S\"[a]\" >log &&\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies \"[a]\" >log &&\n \ttest_cmp log full-log\n '\n \n-- \n2.45.2\n\n"},{"id":"512223","messageId":"20250211085028.3923875-10-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 09/10] diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:21Z","receivedAt":"2025-02-11T08:51:03Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"For less experienced users --patch-{grep,modifies} should be easier to\nunderstand than just -S or -G. By mentioning the long argument names in\nthe help messages we save those users from having to search the list of\noptions for an explanation of what -S or -G stand for.\n---\n diff.c | 4 ++--\n diff.h | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex ac2cd..a9e78 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5871,10 +5871,10 @@ struct option *add_diff_options(const struct option *opts,\n \t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\n-\t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n+\t\t\t N_(\"show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep\"),\n \t\t\t DIFF_PICKAXE_ALL, PARSE_OPT_NONEG),\n \t\tOPT_BIT_F(0, \"pickaxe-regex\", &options->pickaxe_opts,\n-\t\t\t N_(\"treat in -S as extended POSIX regular expression\"),\n+\t\t\t N_(\"treat in -S/--patch-modifies as extended POSIX regular expression\"),\n \t\t\t DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n \t\t\t N_(\"control the order in which files appear in the output\")),\ndiff --git a/diff.h b/diff.h\nindex ed48a..9ad37 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -613,9 +613,9 @@ void diffcore_fix_diff_index(void);\n \" --patch-modifies=\\n\" \\\n \" find filepair who differ in the number of occurrences of string.\\n\" \\\n \" --pickaxe-grep\\n\" \\\n-\" treat as a regex in the -S argument.\\n\" \\\n+\" treat as a regex in the -S/--patch-modifies argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\n-\" show all files diff when -G or -S is used and hit is found.\\n\" \\\n+\" show all files diff for -G/--patch-grep and -S/--patch-modifies.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n \n int diff_queue_is_empty(struct diff_options *o);\n-- \n2.45.2\n\n"},{"id":"512224","messageId":"20250211085028.3923875-11-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v4 10/10] diff: docs: Use --patch-{grep,modifies} over -G/-S","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-11T08:50:22Z","receivedAt":"2025-02-11T08:51:04Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Long argument names are easier to read, compared to short ones. So\nwhile short arguments are great when you want to type a command quickly,\nthe documentation readability is improved if we use long argument names.\n\nNote for reviewers: All changes are just a replacement of `-G` with\n`--patch-grep` and `-S` with `--patch-modifies`. But as the text was\nreformatted to fit the same width in a few places it might look like\nthere are more changes, if the diff is only line-wise and not word-wise.\n\nThe only an exception are changes in `gitdiffcore.adoc`, where I did\nrephrase a sentence. I've moved introduction of the short versions of\nthe `--patch-{grep,modifies}` into a subsequent paragraph. The reason\nis that I wanted to keep a note on the `-G` mnemonic, and it was awkward\nif I would repeat the short definition twice over a span of two\nparagraphs.\n---\n Documentation/diff-options.adoc | 34 ++++++++++----------\n Documentation/git-blame.adoc | 2 +-\n Documentation/gitdiffcore.adoc | 55 +++++++++++++++++----------------\n 3 files changed, 46 insertions(+), 45 deletions(-)\n\ndiff --git a/Documentation/diff-options.adoc b/Documentation/diff-options.adoc\nindex 07413d..c9f7c 100644\n--- a/Documentation/diff-options.adoc\n+++ b/Documentation/diff-options.adoc\n@@ -658,8 +658,8 @@ renamed entries cannot appear if detection for those types is disabled.\n It is useful when you're looking for an exact block of code (like a\n struct), and want to know the history of that block since it first\n came into being: use the feature iteratively to feed the interesting\n-block in the preimage back into `-S`, and keep going until you get the\n-very first version of the block.\n+block in the preimage back into `--patch-modifies`, and keep going until\n+you get the very first version of the block.\n +\n Binary files are searched as well.\n \n@@ -668,9 +668,9 @@ Binary files are searched as well.\n \tLook for differences whose patch text contains added/removed\n \tlines that match __.\n +\n-To illustrate the difference between `-S` `--pickaxe-regex` and\n-`-G`, consider a commit with the following diff in the same\n-file:\n+To illustrate the difference between `--patch-modifies=\n+--pickaxe-regex` and `--patch-grep=`, consider a commit with the\n+following diff in the same file:\n +\n ----\n + return frotz(nitfol, two->ptr, 1, 0);\n@@ -678,9 +678,9 @@ file:\n - hit = frotz(nitfol, mf2.ptr, 1, 0);\n ----\n +\n-While `git log -G\"frotz\\(nitfol\"` will show this commit, `git log\n--S\"frotz\\(nitfol\" --pickaxe-regex` will not (because the number of\n-occurrences of that string did not change).\n+While `git log --patch-grep=\"frotz\\(nitfol\"` will show this commit, `git\n+log --patch-modifies=\"frotz\\(nitfol\" --pickaxe-regex` will not (because the\n+number of occurrences of that string did not change).\n +\n Unless `--text` is supplied patches of binary files without a textconv\n filter will be ignored.\n@@ -689,22 +689,22 @@ See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more\n information.\n \n `--find-object=`::\n-\tLook for differences that change the number of occurrences of\n-\tthe specified object. Similar to `-S`, just the argument is different\n-\tin that it doesn't search for a specific string but for a specific\n-\tobject id.\n+\tLook for differences that change the number of occurrences of the\n+\tspecified object. Similar to `--patch-modifies`, just the argument\n+\tis different in that it doesn't search for a specific string but\n+\tfor a specific object id.\n +\n The object can be a blob or a submodule commit. It implies the `-t` option in\n `git-log` to also find trees.\n \n `--pickaxe-all`::\n-\tWhen `-S` or `-G` finds a change, show all the changes in that\n-\tchangeset, not just the files that contain the change\n-\tin __.\n+\tWhen `--patch-modifies` or `--patch-grep` finds a change, show all\n+\tthe changes in that changeset, not just the files that contain the\n+\tchange in __.\n \n `--pickaxe-regex`::\n-\tTreat the __ given to `-S` as an extended POSIX regular\n-\texpression to match.\n+\tTreat the __ given to `--patch-modifies` as an extended\n+\tPOSIX regular expression to match.\n \n endif::git-format-patch[]\n \ndiff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc\nindex f75ed..10736a 100644\n--- a/Documentation/git-blame.adoc\n+++ b/Documentation/git-blame.adoc\n@@ -41,7 +41,7 @@ a text string in the diff. A small example of the pickaxe interface\n that searches for `blame_usage`:\n \n -----------------------------------------------------------------------------\n-$ git log --pretty=oneline -S'blame_usage'\n+$ git log --pretty=oneline --patch-modifies='blame_usage'\n 5040f17eba15504bad66b14a645bddd9b015ebb7 blame -S \n ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output\n -----------------------------------------------------------------------------\ndiff --git a/Documentation/gitdiffcore.adoc b/Documentation/gitdiffcore.adoc\nindex e934b9..e7f98 100644\n--- a/Documentation/gitdiffcore.adoc\n+++ b/Documentation/gitdiffcore.adoc\n@@ -245,33 +245,34 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. `--patch-modifies=` (`-S` for short) and\n-`--patch-grep=` (`-G` for short) are used to specify\n-different ways these strings are sought.\n-\n-`-S` detects filepairs whose preimage and postimage\n-have different number of occurrences of the specified __.\n-By definition, it will not detect in-file moves. Also, when a\n-changeset moves a file wholesale without affecting the interesting\n-string, diffcore-rename kicks in as usual, and `-S` omits the filepair\n-(since the number of occurrences of that string didn't change in that\n-rename-detected filepair). When used with `--pickaxe-regex`, treat\n-the __ as an extended POSIX regular expression to match,\n-instead of a literal string.\n-\n-`-G` (mnemonic: grep) detects filepairs whose textual diff has\n-an added or a deleted line that matches the given __. This\n-means that it will detect in-file (or what rename-detection considers\n-the same file) moves, which is noise. The implementation runs diff\n-twice and greps, and this can be quite expensive. To speed things up,\n-binary files without textconv filters will be ignored.\n-\n-When `-S` or `-G` are used without `--pickaxe-all`, only filepairs\n-that match their respective criterion are kept in the output. When\n-`--pickaxe-all` is used, if even one filepair matches their respective\n-criterion in a changeset, the entire changeset is kept. This behavior\n-is designed to make reviewing changes in the context of the whole\n-changeset easier.\n+way. `--patch-modifies=` and `--patch-grep=` are used\n+to specify different ways these strings are sought.\n+\n+`--patch-modifies=` (`-S` for short) detects filepairs\n+whose preimage and postimage have different number of occurrences of\n+the specified __. By definition, it will not detect in-file\n+moves. Also, when a changeset moves a file wholesale without\n+affecting the interesting string, diffcore-rename kicks in as usual,\n+and `--patch-modifies` omits the filepair (since the number of\n+occurrences of that string didn't change in that rename-detected\n+filepair). When used with `--pickaxe-regex`, treat the __ as\n+an extended POSIX regular expression to match, instead of a literal\n+string.\n+\n+`--patch-grep=` (`-G` for short, mnemonic: grep) detects\n+filepairs whose textual diff has an added or a deleted line that\n+matches the given regular expression. This means that it will detect\n+in-file (or what rename-detection considers the same file) moves,\n+which is noise. The implementation runs diff twice and greps, and\n+this can be quite expensive. To speed things up, binary files without\n+textconv filters will be ignored.\n+\n+When `--patch-modifies` or `--patch-grep` are used without\n+`--pickaxe-all`, only filepairs that match their respective criterion\n+are kept in the output. When `--pickaxe-all` is used, if even one\n+filepair matches their respective criterion in a changeset, the entire\n+changeset is kept. This behavior is designed to make reviewing\n+changes in the context of the whole changeset easier.\n \n diffcore-order: For Sorting the Output Based on Filenames\n ---------------------------------------------------------\n-- \n2.45.2\n\n"},{"id":"512247","messageId":"xmqqwmdwxsmz.fsf@gitster.g","threadId":"62909","inReplyTo":"20250211085028.3923875-1-illia.bobyr@gmail.com","subject":"Re: [PATCH v4 0/10] Long names for `git log -S` and `git log -G`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-11T18:07:00Z","receivedAt":"2025-02-11T18:07:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> I've split the big change from v3 [1] into multiple, mostly independent patches\n> to make it easier to review and merge each one separately.\n>\n> [1] https://lore.kernel.org/git/20250206014324.1839232-1-illia.bobyr@gmail.com/\n>\n> Patches 1 through 4 are fixing minor bugs and inconsistencies.\n>\n> Patch 5 contains updates gitdiffcore to use same placeholder names as the rest\n> of the code.\n>\n> Patch 6 contains a minimum change to add long versions of -S and -G.\n>\n> Patch 7 adds bash completion support.\n>\n> Patches 8 through 10 increase usage of the long argument versions in tests, CLI\n> help and docs respectively.\n>\n> Please, let me know if you prefer it split in a different way, or reorder the\n> changes.\n\nWhen you base your patch on a different base than 'master' (or if\nthe previous iteration of the topic has already been queued in my\ntree, then the commit used as the base to queue the topic), please\nmake sure you state it clearly. \n\nThis iteration seems to apply on none of bc204b74 (The seventh\nbatch, 2025-02-03), on top of which the previous round dcc02caba2\n(ib/diff-S-G-with-longhand) has been queued, or any of the recent\ntips of 'master', like 388218fa (The ninth batch, 2025-02-10) or\n9520f7d9 (The eighth batch, 2025-02-06), so I cannot look at it.\n\n> I was not sure if I should include a reference to the previous version of the\n> patch into the next reroll. It seems that\n> `Documentation/MyFirstContribution.adoc` suggests so. But it creates very long\n> threads. And I've noticed that not everyone is doing it.\n\nAlmost everybody does so, actually.\n\nTaking a topic that has 5 iterations, each about ~20 patches, as an\nexample:\n\nhttps://lore.kernel.org/git/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im/\n\nit is perfectly clear and easy to nagivate from the list of messages\nwhat discussions we had in previous iterations.\n\n> Reply to review notes ...\n\nIt is more customary to Reply-all directly to review messages,\ninstead of sending new round of patches. When the cover letter of a\nnew iteration is sent as a response to the cover letter of the\nprevious iteration, readers can find the previous discussion\nmessages.\n\nThanks.\n"},{"id":"512269","messageId":"081272b5-b035-47a6-a951-eb923a9a5833@gmail.com","threadId":"62909","inReplyTo":"xmqqseoqiybi.fsf@gitster.g","subject":"Re: [PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:29Z","receivedAt":"2025-02-12T03:26:32Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"On 2/6/25 12:59, Junio C Hamano wrote:\n > Illia Bobyr writes:\n >\n >> Most arguments have both short and long versions.  Long versions are\n >> easier to read, especially in scripts and command history.\n >>\n >> Tests that check just the option parsing are duplicated to check both\n >> short and long argument options.  But more complex tests are updated to\n >> use the long argument in order to improve the test readability.\n >\n > While checking both may be a prudent thing to do, because the \"-S\"\n > option and the \"-G\" option have been there with us almost since the\n > beginning of time, the swapping all existing use of them with the\n > longhand is rather unwelcome and needless churn, I would have to\n > say.\n\nMy thinking is that as long version names improve readability, it also \napplies\nto the test code.  When I see a short option, I often have to check the \nmanual\nto remember what exactly does it do.  Even for \"-S\"/\"-G\" I find myself \nsometimes\nconfused as to which of the two does what exactly.  While the \"grep\" \nmnemonic\nhelps, I do not always remember it.\n\nBut, I think, I understand your point of view as well.\n\nIn v5, patch 5 contains a relatively minimum amount of changes that add long\nalternatives for \"-S\" and \"-G\" just to the command line parsing.\n\nI do not have your experience with assessing the churn, but if my \nargument about\nthe readability changes your mind, I've moved the rest of the updates into\nseparate patches, at the end of the chain.  Patches 8 through 10. Making it\neasier to discuss them in smaller chunks, if you wish so.  But also, I \nassume,\nit should be easy for you to ignore those, if you do not want to include \nthem?\n\n > [...]\n >\n >> diff --git a/Documentation/gitdiffcore.txt \nb/Documentation/gitdiffcore.txt\n >> index 642c5..e4b18 100644\n >> --- a/Documentation/gitdiffcore.txt\n >> +++ b/Documentation/gitdiffcore.txt\n >> @@ -245,33 +245,35 @@ diffcore-pickaxe: For Detecting \nAddition/Deletion of Specified String\n >>\n >>  This transformation limits the set of filepairs to those that change\n >>  specified strings between the preimage and the postimage in a certain\n >> -way.  -S and -G options are used to\n >> +way.  --patch-modifies= and\n >> +--patch-grep= options are used to specify\n >> +different ways these strings are sought.\n >\n > This is worse.  Here is the first part that describes the pickaxe,\n > so mentioning both may be more appropriate; showing only the\n > longhand nobody is familiar with (yet) does not make any sense.\n >\n >     ... certain way. `--patch-modifies=`\n >     (`-S` for short) and \n`--patch-grep=`\n >     (`-G` for short) are used to ...\n >\n > Once establishing the equivalence between the longhand and the\n > shorthand for these two options, we do not have to churn the\n > existing text at all.\n\nApplied your suggestion.\n\nI guess one difference in the way you look at it, is that you default to the\nshort version when you can.  While I default to the long one, as I \nassume it is\neasier to understand.  Someone not that intimately familiar with git \nmight need\nto go to the previous paragraph to recall what \"-S\" and \"-G\" are, while \nif they\nare spelled as \"--patch-modifies\" and \"--patch-grep\", it might be less\nnecessary.  So, the argument is that while we are reducing the diff, we \nmight\nalso be reducing the improvement in readability.\n\nBeing the author, I could also be biased when assessing how much more \nreadable\n\"--patch-modifies\" and \"--patch-grep\" are compared to \"-S\" and \"-G\".\n\n >> diff --git a/diff.c b/diff.c\n >> index d28b41..09beb 100644\n >> --- a/diff.c\n >> +++ b/diff.c\n >> @@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options \n*options)\n >>\n >>      if (HAS_MULTI_BITS(options->pickaxe_opts & \nDIFF_PICKAXE_KINDS_MASK))\n >>          die(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n >> -            \"-G\", \"-S\", \"--find-object\");\n >> +            \"-G/--patch-grep\", \"-S/--patch-modifies\", \"--find-object\");\n >>\n >>      if (HAS_MULTI_BITS(options->pickaxe_opts & \nDIFF_PICKAXE_KINDS_G_REGEX_MASK))\n >>          die(_(\"options '%s' and '%s' cannot be used together, use \n'%s' with '%s'\"),\n >> -            \"-G\", \"--pickaxe-regex\", \"--pickaxe-regex\", \"-S\");\n >> +            \"-G/--patch-grep\", \"--pickaxe-regex\",\n >> +                        \"--pickaxe-regex\", \"-S/--patch-modifies\");\n >>\n >>      if (HAS_MULTI_BITS(options->pickaxe_opts & \nDIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))\n >>          die(_(\"options '%s' and '%s' cannot be used together, use \n'%s' with '%s' and '%s'\"),\n >> -            \"--pickaxe-all\", \"--find-object\", \"--pickaxe-all\", \n\"-G\", \"-S\");\n >> +            \"--pickaxe-all\", \"--find-object\",\n >> +                        \"--pickaxe-all\", \"-G/--patch-grep\", \n\"-S/--patch-modifies\");\n >\n > The message change looks fine; the indentation is broken.\n >\n > .git/rebase-apply/patch:184: indent with spaces.\n >                         \"--pickaxe-regex\", \"-S/--patch-modifies\");\n > .git/rebase-apply/patch:190: indent with spaces.\n >                         \"--pickaxe-all\", \"-G/--patch-grep\", \n\"-S/--patch-modifies\");\n > warning: 2 lines applied after fixing whitespace errors.\n > Applying: diff: --patch-{modifies,grep} arg names for -S and -G\n >\n > These alone do not require a new iteration, as \"git am --whitespace=fix\"\n > already corrected them.\n\nSorry about this.  I did check the indentation manually, but did not use a\ntool.  Reconfigured my editor to use tabs now.\n\n >> -        OPT_CALLBACK_F('S', NULL, options, N_(\"\"),\n >> +        OPT_CALLBACK_F('S', \"patch-modifies\", options, N_(\"\"),\n >> -        OPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n >> +        OPT_CALLBACK_F('G', \"patch-grep\", options, N_(\"\"),\n >\n > OK.  NOte that this says .  We may want to have a separate \nclean-up\n > patch so that Documentation/gitdifcore.txt that used \n > and the placeholder used here match.\n\nMakes sense.\nI've added this fix as patch 5 in v5.\nI've reformatted paragraphs in gitdifcore.adoc that were affected.\nLet me know if you do not want me to reformat it, and just keep shorter \nlines as\nis.\n\n >> -                   N_(\"look for differences that change the number \nof occurrences of the specified regex\"),\n >> +                   N_(\"look for differences where a patch contains \nthe specified regex\"),\n >\n > This is an unrelated change that should not be in this patch.  If\n > you want to modify it, please do it in a separate clean-up patch,\n > just like the above vs change.\n\nSplit it into patch 2 in v5.\n\n >> -              N_(\"show all changes in the changeset with -S or -G\"),\n >> +              N_(\"show all changes in the changeset with \n-S/--patch-modifies or -G/--patch-grep\"),\n >\n > This line is meant to be shown when the user requests list of\n > options and their meanings.  Growing the message from 47 columns or\n > so to 78 columns would make it wider than terminals when these\n > messages are indented.  Because earlier entries in this array have\n > already established the equivalence between the shorthand and the\n > longhand, I do not think the output is understandable without this\n > change.\n\nA description for \"-S\" is already 81 characters long:\n\nN_(\"look for differences that change the number of occurrences of the \nspecified regex\"),\n\nSo I was assuming if I grow another description to 77 characters, it is \nstill OK.\nWhile one can find the correspondence between \"-S\" and \"--patch-modifies\" by\nreading the \"-S\" description, in my mind, the same argument applies as \nto the\ntest readability - it just makes it a bit easier for the reader.\n\nThis change is now part of a much smaller patch 9 in v5, which is only about\nadding longer alternatives to the CLI help messages that currently \ncontain only\n\"-G\" and \"-S\".  This way you can decide if you want it or not as a complete\nunit.  Or if you want me to change it in some way, we can discuss it \nseparately\nfrom the rest of the changes.\n\nBy the way, I must admit I can not find a way to look at a help message\ngenerated from these strings.  Running `git diff -h` shows a message from\n\n`diff.h` and running `git diff --help` shows the man page.\n\nThank you.\n\n"},{"id":"512270","messageId":"20250212032657.1807939-1-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250206014324.1839232-1-illia.bobyr@gmail.com","subject":"[PATCH v5 00/10] Long names for `git log -S` and `git log -G`","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:44Z","receivedAt":"2025-02-12T03:27:08Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Rebased v4 [1] on top of `master`.\nI've accidentally published v4 based on `next`.\n\n[1] https://lore.kernel.org/git/20250211085028.3923875-1-illia.bobyr@gmail.com/\n\nOther than the rebase, it is identical to v4, details below.\n\nSplit the big change from v3 [1] into multiple, mostly independent patches, to\nmake it easier to review and merge each one separately.\n\nPatches 1 through 4 are fixing minor bugs and inconsistencies.\n\nPatch 5 contains updates gitdiffcore to use same placeholder names as the rest\nof the code.\n\nPatch 6 contains a minimum change to add long versions of -S and -G.\n\nPatch 7 adds bash completion support.\n\nPatches 8 through 10 increase usage of the long argument versions in tests, CLI\nhelp and docs respectively.\n\nPlease, let me know if you prefer it split in a different way, or reorder the\nchanges.\n\nIllia Bobyr (10):\n t/t4209-log-pickaxe: Naming typo: -G takes a regex\n diff: -G description: Correct copy/paste error\n diff: short help: Correct -S description\n diff: short help: Add -G and --pickaxe-grep\n docs: gitdiffcore: -G and -S: Use regex/string placeholders\n diff: --patch-{grep,modifies} arg names for -G and -S\n completion: Support --patch-{grep,modifies}\n diff: test: Use --patch-{grep,modifies} over -G/-S\n diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}\n diff: docs: Use --patch-{grep,modifies} over -G/-S\n\n Documentation/diff-options.txt | 36 +++++----\n Documentation/git-blame.txt | 2 +-\n Documentation/gitdiffcore.txt | 55 ++++++-------\n contrib/completion/git-completion.bash | 11 ++-\n diff.c | 18 +++--\n diff.h | 11 ++-\n t/t4062-diff-pickaxe.sh | 8 +-\n t/t4209-log-pickaxe.sh | 106 +++++++++++++++++--------\n 8 files changed, 155 insertions(+), 92 deletions(-)\n\n-- \n2.45.2\n\n"},{"id":"512271","messageId":"20250212032657.1807939-2-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:45Z","receivedAt":"2025-02-12T03:27:14Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Not effect on the test logic, but as \"-G\" argument is a regex it is more\naccurate to use \"regex\" as a dummy argument value rather than \"string\".\nIn all the other case when \"-G\" is passed a dummy value it is spelled as\n\"regex\" rather than as \"string\".\n---\n t/t4209-log-pickaxe.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex a675ac..ed70c 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -89,7 +89,7 @@ test_expect_success 'usage: --no-pickaxe-regex' '\n \ttest_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&\n \ttest_cmp expect actual &&\n \n-\ttest_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&\n+\ttest_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&\n \ttest_cmp expect actual\n '\n \n-- \n2.45.2\n\n"},{"id":"512272","messageId":"20250212032657.1807939-3-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 02/10] diff: -G description: Correct copy/paste error","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:46Z","receivedAt":"2025-02-12T03:27:15Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Current description for -G is incorrect, seems like it was copied from\nthe description for -S.\n---\n diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 019fb..bd9db 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5866,7 +5866,7 @@ struct option *add_diff_options(const struct option *opts,\n \t\t\t N_(\"look for differences that change the number of occurrences of the specified string\"),\n \t\t\t 0, diff_opt_pickaxe_string),\n \t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n-\t\t\t N_(\"look for differences that change the number of occurrences of the specified regex\"),\n+\t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\n \t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n-- \n2.45.2\n\n"},{"id":"512273","messageId":"20250212032657.1807939-5-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 04/10] diff: short help: Add -G and --pickaxe-grep","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:48Z","receivedAt":"2025-02-12T03:27:17Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"-G and --pickaxe-grep seems to be on par with -S and --pickaxe-all that\nare already mentioned.\n---\n diff.h | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.h b/diff.h\nindex 49ece3..787bb 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -606,9 +606,12 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n+\" -G find differences where patch contains the specified regex.\\n\" \\\n \" -S find filepair who differ in the number of occurrences of string.\\n\" \\\n+\" --pickaxe-grep\\n\" \\\n+\" treat as a regex in the -S argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\n-\" show all files diff when -S is used and hit is found.\\n\" \\\n+\" show all files diff when -G or -S is used and hit is found.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n \n int diff_queue_is_empty(struct diff_options *o);\n-- \n2.45.2\n\n"},{"id":"512274","messageId":"20250212032657.1807939-4-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 03/10] diff: short help: Correct -S description","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:47Z","receivedAt":"2025-02-12T03:27:17Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"`-S` shows changes that modify the number of occurrences of the\nspecified string, rather than only those that either completely remove\nit or add it for the first time.\n---\n diff.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.h b/diff.h\nindex 0a566f..49ece3 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -606,7 +606,7 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n-\" -S find filepair whose only one side contains the string.\\n\" \\\n+\" -S find filepair who differ in the number of occurrences of string.\\n\" \\\n \" --pickaxe-all\\n\" \\\n \" show all files diff when -S is used and hit is found.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n-- \n2.45.2\n\n"},{"id":"512275","messageId":"20250212032657.1807939-6-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 05/10] docs: gitdiffcore: -G and -S: Use regex/string placeholders","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:49Z","receivedAt":"2025-02-12T03:27:19Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"In the rest of the documentation (and in the code) we use `regex` and\n`string` as `-G` and `-S` argument placeholders. While\n`regular-expression` and `block-of-text` are a bit easier to read, it is\na bit consistent.\n\nAnd we could assume that everyone who uses git should be able to\nunderstand that a \"string\" and a \"block-of-text\", as well as a \"regex\"\nand \"regular-expression\" are the same thing. So, using a shorter\nversion is also more consistent.\n---\n Documentation/gitdiffcore.txt | 23 +++++++++++------------\n 1 file changed, 11 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\nindex 642c5..0d7d66 100644\n--- a/Documentation/gitdiffcore.txt\n+++ b/Documentation/gitdiffcore.txt\n@@ -245,26 +245,25 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. -S and -G options are used to\n-specify different ways these strings are sought.\n+way. `-S` and `-G` options are used to specify\n+different ways these strings are sought.\n \n-\"-S\" detects filepairs whose preimage and postimage\n-have different number of occurrences of the specified block of text.\n+`-S` detects filepairs whose preimage and postimage\n+have different number of occurrences of the specified __.\n By definition, it will not detect in-file moves. Also, when a\n changeset moves a file wholesale without affecting the interesting\n string, diffcore-rename kicks in as usual, and `-S` omits the filepair\n (since the number of occurrences of that string didn't change in that\n rename-detected filepair). When used with `--pickaxe-regex`, treat\n-the as an extended POSIX regular expression to match,\n+the __ as an extended POSIX regular expression to match,\n instead of a literal string.\n \n-\"-G\" (mnemonic: grep) detects filepairs whose\n-textual diff has an added or a deleted line that matches the given\n-regular expression. This means that it will detect in-file (or what\n-rename-detection considers the same file) moves, which is noise. The\n-implementation runs diff twice and greps, and this can be quite\n-expensive. To speed things up, binary files without textconv filters\n-will be ignored.\n+`-G` (mnemonic: grep) detects filepairs whose textual diff has\n+an added or a deleted line that matches the given __. This\n+means that it will detect in-file (or what rename-detection considers\n+the same file) moves, which is noise. The implementation runs diff\n+twice and greps, and this can be quite expensive. To speed things up,\n+binary files without textconv filters will be ignored.\n \n When `-S` or `-G` are used without `--pickaxe-all`, only filepairs\n that match their respective criterion are kept in the output. When\n-- \n2.45.2\n\n"},{"id":"512276","messageId":"20250212032657.1807939-7-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 06/10] diff: --patch-{grep,modifies} arg names for -G and -S","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:50Z","receivedAt":"2025-02-12T03:27:20Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Most arguments have both short and long versions. Long versions are\neasier to read, especially in scripts and command history.\n\nThis change mostly keeps existing uses of -G and -S as is in the tests,\ndocumentation and help output.\n\nTests that check just the option parsing are duplicated to check both\nshort and long argument options.\n\nSigned-off-by: Illia Bobyr \n---\n Documentation/diff-options.txt | 2 ++\n Documentation/gitdiffcore.txt | 3 ++-\n diff.c | 12 ++++++----\n diff.h | 8 +++++--\n t/t4209-log-pickaxe.sh | 42 ++++++++++++++++++++++++++++++++++\n 5 files changed, 59 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 640eb..07413d 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -650,6 +650,7 @@ Note that not all diffs can feature all types. For instance, copied and\n renamed entries cannot appear if detection for those types is disabled.\n \n `-S`::\n+`--patch-modifies=`::\n \tLook for differences that change the number of occurrences of\n \tthe specified __ (i.e. addition/deletion) in a file.\n \tIntended for the scripter's use.\n@@ -663,6 +664,7 @@ very first version of the block.\n Binary files are searched as well.\n \n `-G`::\n+`--patch-grep=`::\n \tLook for differences whose patch text contains added/removed\n \tlines that match __.\n +\ndiff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\nindex 0d7d66..e934b9 100644\n--- a/Documentation/gitdiffcore.txt\n+++ b/Documentation/gitdiffcore.txt\n@@ -245,7 +245,8 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. `-S` and `-G` options are used to specify\n+way. `--patch-modifies=` (`-S` for short) and\n+`--patch-grep=` (`-G` for short) are used to specify\n different ways these strings are sought.\n \n `-S` detects filepairs whose preimage and postimage\ndiff --git a/diff.c b/diff.c\nindex bd9db..ac2cd 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4877,15 +4877,17 @@ void diff_setup_done(struct diff_options *options)\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n-\t\t\t\"-G\", \"-S\", \"--find-object\");\n+\t\t\t\"-G/--patch-grep\", \"-S/--patch-modifies\", \"--find-object\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_G_REGEX_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s'\"),\n-\t\t\t\"-G\", \"--pickaxe-regex\", \"--pickaxe-regex\", \"-S\");\n+\t\t\t\"-G/--patch-grep\", \"--pickaxe-regex\",\n+\t\t\t\"--pickaxe-regex\", \"-S/--patch-modifies\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_ALL_OBJFIND_MASK))\n \t\tdie(_(\"options '%s' and '%s' cannot be used together, use '%s' with '%s' and '%s'\"),\n-\t\t\t\"--pickaxe-all\", \"--find-object\", \"--pickaxe-all\", \"-G\", \"-S\");\n+\t\t\t\"--pickaxe-all\", \"--find-object\",\n+\t\t\t\"--pickaxe-all\", \"-G/--patch-grep\", \"-S/--patch-modifies\");\n \n \t/*\n \t * Most of the time we can say \"there are changes\"\n@@ -5862,10 +5864,10 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_SET_INT_F(0, \"ita-visible-in-index\", &options->ita_invisible_in_index,\n \t\t\t N_(\"treat 'git add -N' entries as real in the index\"),\n \t\t\t 0, PARSE_OPT_NONEG),\n-\t\tOPT_CALLBACK_F('S', NULL, options, N_(\"\"),\n+\t\tOPT_CALLBACK_F('S', \"patch-modifies\", options, N_(\"\"),\n \t\t\t N_(\"look for differences that change the number of occurrences of the specified string\"),\n \t\t\t 0, diff_opt_pickaxe_string),\n-\t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n+\t\tOPT_CALLBACK_F('G', \"patch-grep\", options, N_(\"\"),\n \t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\ndiff --git a/diff.h b/diff.h\nindex 787bb..ed48a 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -606,8 +606,12 @@ void diffcore_fix_diff_index(void);\n \" try unchanged files as candidate for copy detection.\\n\" \\\n \" -l limit rename attempts up to paths.\\n\" \\\n \" -O reorder diffs according to the .\\n\" \\\n-\" -G find differences where patch contains the specified regex.\\n\" \\\n-\" -S find filepair who differ in the number of occurrences of string.\\n\" \\\n+\" -G\\n\" \\\n+\" --patch-grep=\\n\" \\\n+\" find differences where patch contains the regex.\\n\" \\\n+\" -S\\n\" \\\n+\" --patch-modifies=\\n\" \\\n+\" find filepair who differ in the number of occurrences of string.\\n\" \\\n \" --pickaxe-grep\\n\" \\\n \" treat as a regex in the -S argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex ed70c..ab14b 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -60,24 +60,48 @@ test_expect_success 'usage' '\n \ttest_expect_code 129 git log -S 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-modifies 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 129 git log -G 2>err &&\n \ttest_grep \"switch.*requires a value\" err &&\n \n+\ttest_expect_code 129 git log --patch-grep 2>err &&\n+\ttest_grep \"option.*requires a value\" err &&\n+\n \ttest_expect_code 128 git log -Gregex -Sstring 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log -Gregex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex -Sstring 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --patch-modifies string 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Gregex --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-grep regex --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log -Sstring --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err &&\n \n+\ttest_expect_code 128 git log --patch-modifies string --find-object=HEAD 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n \ttest_expect_code 128 git log --pickaxe-all --find-object=HEAD 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n test_expect_success 'usage: --pickaxe-regex' '\n \ttest_expect_code 128 git log -Gregex --pickaxe-regex 2>err &&\n+\tgrep \"cannot be used together\" err &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --pickaxe-regex 2>err &&\n \tgrep \"cannot be used together\" err\n '\n \n@@ -89,7 +113,13 @@ test_expect_success 'usage: --no-pickaxe-regex' '\n \ttest_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&\n \ttest_cmp expect actual &&\n \n+\ttest_expect_code 128 git log --patch-modifies string --no-pickaxe-regex 2>actual &&\n+\ttest_cmp expect actual &&\n+\n \ttest_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&\n+\ttest_cmp expect actual &&\n+\n+\ttest_expect_code 128 git log --patch-grep regex --no-pickaxe-regex 2>err &&\n \ttest_cmp expect actual\n '\n \n@@ -104,9 +134,13 @@ test_log_icase\texpect_second\t--author person\n test_log_icase\texpect_nomatch\t--author spreon\n \n test_log\texpect_nomatch\t-G picked\n+test_log\texpect_nomatch\t--patch-grep picked\n test_log\texpect_second\t-G Picked\n+test_log\texpect_second\t--patch-grep Picked\n test_log_icase\texpect_nomatch\t-G pickle\n+test_log_icase\texpect_nomatch\t--patch-grep pickle\n test_log_icase\texpect_second\t-G picked\n+test_log_icase\texpect_second\t--patch-grep picked\n \n test_expect_success 'log -G --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n@@ -122,14 +156,22 @@ test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n '\n \n test_log\texpect_nomatch\t-S picked\n+test_log\texpect_nomatch\t--patch-modifies picked\n test_log\texpect_second\t-S Picked\n+test_log\texpect_second\t--patch-modifies Picked\n test_log_icase\texpect_second\t-S picked\n+test_log_icase\texpect_second\t--patch-modifies picked\n test_log_icase\texpect_nomatch\t-S pickle\n+test_log_icase\texpect_nomatch\t--patch-modifies pickle\n \n test_log\texpect_nomatch\t-S p.cked --pickaxe-regex\n+test_log\texpect_nomatch\t--patch-modifies p.cked --pickaxe-regex\n test_log\texpect_second\t-S P.cked --pickaxe-regex\n+test_log\texpect_second\t--patch-modifies P.cked --pickaxe-regex\n test_log_icase\texpect_second\t-S p.cked --pickaxe-regex\n+test_log_icase\texpect_second\t--patch-modifies p.cked --pickaxe-regex\n test_log_icase\texpect_nomatch\t-S p.ckle --pickaxe-regex\n+test_log_icase\texpect_nomatch\t--patch-modifies p.ckle --pickaxe-regex\n \n test_expect_success 'log -S --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-- \n2.45.2\n\n"},{"id":"512277","messageId":"20250212032657.1807939-8-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 07/10] completion: Support --patch-{grep,modifies}","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:51Z","receivedAt":"2025-02-12T03:27:21Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"---\n contrib/completion/git-completion.bash | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 41391..daf335 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1900,6 +1900,7 @@ __git_diff_common_options=\"--stat --numstat --shortstat --summary\n \t\t\t--output= --output-indicator-context=\n \t\t\t--output-indicator-new= --output-indicator-old=\n \t\t\t--ws-error-highlight=\n+\t\t\t--patch-grep= --patch-modifies=\n \t\t\t--pickaxe-all --pickaxe-regex --patch-with-raw\n \"\n \n@@ -2216,7 +2217,7 @@ __git_complete_log_opts ()\n \t\t__git_complete_symbol --cur=\"${cur#:}\" --sfx=\":\"\n \t\treturn\n \t\t;;\n-\t-G,*|-S,*)\n+\t-G,*|--patch-grep,*|-S,*|--patch-modifies,*)\n \t\t__git_complete_symbol\n \t\treturn\n \t\t;;\n@@ -2239,6 +2240,14 @@ __git_complete_log_opts ()\n \t\t__gitcomp \"$__git_diff_algorithms\" \"\" \"${cur##--diff-algorithm=}\"\n \t\treturn\n \t\t;;\n+\t--patch-grep=*)\n+\t\t__git_complete_symbol --pfx=\"--patch-grep=\" --cur=\"${cur#--patch-grep=}\"\n+\t\treturn\n+\t\t;;\n+\t--patch-modifies=*)\n+\t\t__git_complete_symbol --pfx=\"--patch-modifies=\" --cur=\"${cur#--patch-modifies=}\"\n+\t\treturn\n+\t\t;;\n \t--submodule=*)\n \t\t__gitcomp \"$__git_diff_submodule_formats\" \"\" \"${cur##--submodule=}\"\n \t\treturn\n-- \n2.45.2\n\n"},{"id":"512278","messageId":"20250212032657.1807939-9-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 08/10] diff: test: Use --patch-{grep,modifies} over -G/-S","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:52Z","receivedAt":"2025-02-12T03:27:23Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Long argument names are easier to read, compared to short ones. So\nwhile short arguments are great when you want to type a command quickly,\ntests are more readable if we use long argument names.\n\nThere are still test that verify that both short and long arguments work\ninterchangeably when parsing the arguments.\n\nTests where the focus is not on the argument names are updated to use\nlong argument names.\n---\n t/t4062-diff-pickaxe.sh | 8 +++---\n t/t4209-log-pickaxe.sh | 62 ++++++++++++++++++++---------------------\n 2 files changed, 35 insertions(+), 35 deletions(-)\n\ndiff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh\nindex 8ad3d7..805e0f 100755\n--- a/t/t4062-diff-pickaxe.sh\n+++ b/t/t4062-diff-pickaxe.sh\n@@ -16,13 +16,13 @@ test_expect_success setup '\n '\n \n # OpenBSD only supports up to 255 repetitions, so repeat twice for 64*64=4096.\n-test_expect_success '-G matches' '\n-\tgit diff --name-only -G \"^(0{64}){64}$\" HEAD^ >out &&\n+test_expect_success '--patch-grep matches' '\n+\tgit diff --name-only --patch-grep \"^(0{64}){64}$\" HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \n-test_expect_success '-S --pickaxe-regex' '\n-\tgit diff --name-only -S0 --pickaxe-regex HEAD^ >out &&\n+test_expect_success '--patch-modifies --pickaxe-regex' '\n+\tgit diff --name-only --patch-modifies 0 --pickaxe-regex HEAD^ >out &&\n \ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \ndiff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\nindex ab14b..5f4d6 100755\n--- a/t/t4209-log-pickaxe.sh\n+++ b/t/t4209-log-pickaxe.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='log --grep/--author/--regexp-ignore-case/-S/-G'\n+test_description='log --grep/--author/--regexp-ignore-case/--patch-{modifies,grep}'\n \n . ./test-lib.sh\n \n@@ -142,15 +142,15 @@ test_log_icase\texpect_nomatch\t--patch-grep pickle\n test_log_icase\texpect_second\t-G picked\n test_log_icase\texpect_second\t--patch-grep picked\n \n-test_expect_success 'log -G --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Gfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-grep foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -G --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-grep --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Gfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-grep foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n@@ -173,20 +173,20 @@ test_log_icase\texpect_second\t--patch-modifies p.cked --pickaxe-regex\n test_log_icase\texpect_nomatch\t-S p.ckle --pickaxe-regex\n test_log_icase\texpect_nomatch\t--patch-modifies p.ckle --pickaxe-regex\n \n-test_expect_success 'log -S --textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\ttest_must_fail git -c diff.test.textconv=missing log -Sfoo &&\n+\ttest_must_fail git -c diff.test.textconv=missing log --patch-modifies foo &&\n \trm .gitattributes\n '\n \n-test_expect_success 'log -S --no-textconv (missing textconv tool)' '\n+test_expect_success 'log --patch-modifies --no-textconv (missing textconv tool)' '\n \techo \"* diff=test\" >.gitattributes &&\n-\tgit -c diff.test.textconv=missing log -Sfoo --no-textconv >actual &&\n+\tgit -c diff.test.textconv=missing log --patch-modifies foo --no-textconv >actual &&\n \ttest_cmp expect_nomatch actual &&\n \trm .gitattributes\n '\n \n-test_expect_success 'setup log -[GS] plain & regex' '\n+test_expect_success 'setup log --patch{-modifies,-grep} plain & regex' '\n \ttest_create_repo GS-plain &&\n \ttest_commit -C GS-plain --append A data.txt \"a\" &&\n \ttest_commit -C GS-plain --append B data.txt \"a a\" &&\n@@ -201,31 +201,31 @@ test_expect_success 'setup log -[GS] plain & regex' '\n \tgit -C GS-plain log >full-log\n '\n \n-test_expect_success 'log -G trims diff new/old [-+]' '\n-\tgit -C GS-plain log -G\"[+-]a\" >log &&\n+test_expect_success 'log --patch-grep trims diff new/old [-+]' '\n+\tgit -C GS-plain log --patch-grep \"[+-]a\" >log &&\n \ttest_must_be_empty log &&\n-\tgit -C GS-plain log -G\"^a\" >log &&\n+\tgit -C GS-plain log --patch-grep \"^a\" >log &&\n \ttest_cmp log A-to-B-then-E-log\n '\n \n-test_expect_success 'log -S is not a regex, but -S --pickaxe-regex is' '\n-\tgit -C GS-plain log -S\"a\" >log &&\n+test_expect_success 'log --patch-modifies is not a regex, but --patch-modifies --pickaxe-regex is' '\n+\tgit -C GS-plain log --patch-modifies \"a\" >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[a]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" >log &&\n \ttest_must_be_empty log &&\n \n-\tgit -C GS-plain log -S\"[a]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[a]\" --pickaxe-regex >log &&\n \ttest_cmp log A-to-B-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" >log &&\n \ttest_cmp log D-then-E-log &&\n \n-\tgit -C GS-plain log -S\"[b]\" --pickaxe-regex >log &&\n+\tgit -C GS-plain log --patch-modifies \"[b]\" --pickaxe-regex >log &&\n \ttest_cmp log C-to-D-then-E-log\n '\n \n-test_expect_success 'setup log -[GS] binary & --text' '\n+test_expect_success 'setup log --patch{-modifies,-grep} binary & --text' '\n \ttest_create_repo GS-bin-txt &&\n \ttest_commit -C GS-bin-txt --printf A data.bin \"a\\na\\0a\\n\" &&\n \ttest_commit -C GS-bin-txt --append --printf B data.bin \"a\\na\\0a\\n\" &&\n@@ -233,36 +233,36 @@ test_expect_success 'setup log -[GS] binary & --text' '\n \tgit -C GS-bin-txt log >full-log\n '\n \n-test_expect_success 'log -G ignores binary files' '\n-\tgit -C GS-bin-txt log -Ga >log &&\n+test_expect_success 'log --patch-grep ignores binary files' '\n+\tgit -C GS-bin-txt log --patch-grep a >log &&\n \ttest_must_be_empty log\n '\n \n-test_expect_success 'log -G looks into binary files with -a' '\n-\tgit -C GS-bin-txt log -a -Ga >log &&\n+test_expect_success 'log --patch-grep looks into binary files with -a' '\n+\tgit -C GS-bin-txt log -a --patch-grep a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -G looks into binary files with textconv filter' '\n+test_expect_success 'log --patch-grep looks into binary files with textconv filter' '\n \ttest_when_finished \"rm GS-bin-txt/.gitattributes\" &&\n \t(\n \t\tcd GS-bin-txt &&\n \t\techo \"* diff=bin\" >.gitattributes &&\n-\t\tgit -c diff.bin.textconv=cat log -Ga >../log\n+\t\tgit -c diff.bin.textconv=cat log --patch-grep a >../log\n \t) &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S looks into binary files' '\n-\tgit -C GS-bin-txt log -Sa >log &&\n+test_expect_success 'log --patch-modifies looks into binary files' '\n+\tgit -C GS-bin-txt log --patch-modifies a >log &&\n \ttest_cmp log full-log\n '\n \n-test_expect_success 'log -S --pickaxe-regex looks into binary files' '\n-\tgit -C GS-bin-txt log --pickaxe-regex -Sa >log &&\n+test_expect_success 'log --patch-modifies --pickaxe-regex looks into binary files' '\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies a >log &&\n \ttest_cmp log full-log &&\n \n-\tgit -C GS-bin-txt log --pickaxe-regex -S\"[a]\" >log &&\n+\tgit -C GS-bin-txt log --pickaxe-regex --patch-modifies \"[a]\" >log &&\n \ttest_cmp log full-log\n '\n \n-- \n2.45.2\n\n"},{"id":"512279","messageId":"20250212032657.1807939-10-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 09/10] diff: --pickaxe-{all,regex} help: Add --patch-{grep,modifies}","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:53Z","receivedAt":"2025-02-12T03:27:24Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"For less experienced users --patch-{grep,modifies} should be easier to\nunderstand than just -S or -G. By mentioning the long argument names in\nthe help messages we save those users from having to search the list of\noptions for an explanation of what -S or -G stand for.\n---\n diff.c | 4 ++--\n diff.h | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex ac2cd..a9e78 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5871,10 +5871,10 @@ struct option *add_diff_options(const struct option *opts,\n \t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n \t\t\t 0, diff_opt_pickaxe_regex),\n \t\tOPT_BIT_F(0, \"pickaxe-all\", &options->pickaxe_opts,\n-\t\t\t N_(\"show all changes in the changeset with -S or -G\"),\n+\t\t\t N_(\"show all changes in the changeset with -S/--patch-modifies or -G/--patch-grep\"),\n \t\t\t DIFF_PICKAXE_ALL, PARSE_OPT_NONEG),\n \t\tOPT_BIT_F(0, \"pickaxe-regex\", &options->pickaxe_opts,\n-\t\t\t N_(\"treat in -S as extended POSIX regular expression\"),\n+\t\t\t N_(\"treat in -S/--patch-modifies as extended POSIX regular expression\"),\n \t\t\t DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n \t\t\t N_(\"control the order in which files appear in the output\")),\ndiff --git a/diff.h b/diff.h\nindex ed48a..9ad37 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -613,9 +613,9 @@ void diffcore_fix_diff_index(void);\n \" --patch-modifies=\\n\" \\\n \" find filepair who differ in the number of occurrences of string.\\n\" \\\n \" --pickaxe-grep\\n\" \\\n-\" treat as a regex in the -S argument.\\n\" \\\n+\" treat as a regex in the -S/--patch-modifies argument.\\n\" \\\n \" --pickaxe-all\\n\" \\\n-\" show all files diff when -G or -S is used and hit is found.\\n\" \\\n+\" show all files diff for -G/--patch-grep and -S/--patch-modifies.\\n\" \\\n \" -a --text treat all files as text.\\n\"\n \n int diff_queue_is_empty(struct diff_options *o);\n-- \n2.45.2\n\n"},{"id":"512280","messageId":"20250212032657.1807939-11-illia.bobyr@gmail.com","threadId":"62909","inReplyTo":"20250212032657.1807939-1-illia.bobyr@gmail.com","subject":"[PATCH v5 10/10] diff: docs: Use --patch-{grep,modifies} over -G/-S","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:26:54Z","receivedAt":"2025-02-12T03:27:25Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"Long argument names are easier to read, compared to short ones. So\nwhile short arguments are great when you want to type a command quickly,\nthe documentation readability is improved if we use long argument names.\n\nNote for reviewers: All changes are just a replacement of `-G` with\n`--patch-grep` and `-S` with `--patch-modifies`. But as the text was\nreformatted to fit the same width in a few places it might look like\nthere are more changes, if the diff is only line-wise and not word-wise.\n\nThe only an exception are changes in `gitdiffcore.adoc`, where I did\nrephrase a sentence. I've moved introduction of the short versions of\nthe `--patch-{grep,modifies}` into a subsequent paragraph. The reason\nis that I wanted to keep a note on the `-G` mnemonic, and it was awkward\nif I would repeat the short definition twice over a span of two\nparagraphs.\n---\n Documentation/diff-options.txt | 34 ++++++++++-----------\n Documentation/git-blame.txt | 2 +-\n Documentation/gitdiffcore.txt | 55 +++++++++++++++++-----------------\n 3 files changed, 46 insertions(+), 45 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 07413d..c9f7c 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -658,8 +658,8 @@ renamed entries cannot appear if detection for those types is disabled.\n It is useful when you're looking for an exact block of code (like a\n struct), and want to know the history of that block since it first\n came into being: use the feature iteratively to feed the interesting\n-block in the preimage back into `-S`, and keep going until you get the\n-very first version of the block.\n+block in the preimage back into `--patch-modifies`, and keep going until\n+you get the very first version of the block.\n +\n Binary files are searched as well.\n \n@@ -668,9 +668,9 @@ Binary files are searched as well.\n \tLook for differences whose patch text contains added/removed\n \tlines that match __.\n +\n-To illustrate the difference between `-S` `--pickaxe-regex` and\n-`-G`, consider a commit with the following diff in the same\n-file:\n+To illustrate the difference between `--patch-modifies=\n+--pickaxe-regex` and `--patch-grep=`, consider a commit with the\n+following diff in the same file:\n +\n ----\n + return frotz(nitfol, two->ptr, 1, 0);\n@@ -678,9 +678,9 @@ file:\n - hit = frotz(nitfol, mf2.ptr, 1, 0);\n ----\n +\n-While `git log -G\"frotz\\(nitfol\"` will show this commit, `git log\n--S\"frotz\\(nitfol\" --pickaxe-regex` will not (because the number of\n-occurrences of that string did not change).\n+While `git log --patch-grep=\"frotz\\(nitfol\"` will show this commit, `git\n+log --patch-modifies=\"frotz\\(nitfol\" --pickaxe-regex` will not (because the\n+number of occurrences of that string did not change).\n +\n Unless `--text` is supplied patches of binary files without a textconv\n filter will be ignored.\n@@ -689,22 +689,22 @@ See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more\n information.\n \n `--find-object=`::\n-\tLook for differences that change the number of occurrences of\n-\tthe specified object. Similar to `-S`, just the argument is different\n-\tin that it doesn't search for a specific string but for a specific\n-\tobject id.\n+\tLook for differences that change the number of occurrences of the\n+\tspecified object. Similar to `--patch-modifies`, just the argument\n+\tis different in that it doesn't search for a specific string but\n+\tfor a specific object id.\n +\n The object can be a blob or a submodule commit. It implies the `-t` option in\n `git-log` to also find trees.\n \n `--pickaxe-all`::\n-\tWhen `-S` or `-G` finds a change, show all the changes in that\n-\tchangeset, not just the files that contain the change\n-\tin __.\n+\tWhen `--patch-modifies` or `--patch-grep` finds a change, show all\n+\tthe changes in that changeset, not just the files that contain the\n+\tchange in __.\n \n `--pickaxe-regex`::\n-\tTreat the __ given to `-S` as an extended POSIX regular\n-\texpression to match.\n+\tTreat the __ given to `--patch-modifies` as an extended\n+\tPOSIX regular expression to match.\n \n endif::git-format-patch[]\n \ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex b1d7fb..0f21d3 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -41,7 +41,7 @@ a text string in the diff. A small example of the pickaxe interface\n that searches for `blame_usage`:\n \n -----------------------------------------------------------------------------\n-$ git log --pretty=oneline -S'blame_usage'\n+$ git log --pretty=oneline --patch-modifies='blame_usage'\n 5040f17eba15504bad66b14a645bddd9b015ebb7 blame -S \n ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output\n -----------------------------------------------------------------------------\ndiff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\nindex e934b9..e7f98 100644\n--- a/Documentation/gitdiffcore.txt\n+++ b/Documentation/gitdiffcore.txt\n@@ -245,33 +245,34 @@ diffcore-pickaxe: For Detecting Addition/Deletion of Specified String\n \n This transformation limits the set of filepairs to those that change\n specified strings between the preimage and the postimage in a certain\n-way. `--patch-modifies=` (`-S` for short) and\n-`--patch-grep=` (`-G` for short) are used to specify\n-different ways these strings are sought.\n-\n-`-S` detects filepairs whose preimage and postimage\n-have different number of occurrences of the specified __.\n-By definition, it will not detect in-file moves. Also, when a\n-changeset moves a file wholesale without affecting the interesting\n-string, diffcore-rename kicks in as usual, and `-S` omits the filepair\n-(since the number of occurrences of that string didn't change in that\n-rename-detected filepair). When used with `--pickaxe-regex`, treat\n-the __ as an extended POSIX regular expression to match,\n-instead of a literal string.\n-\n-`-G` (mnemonic: grep) detects filepairs whose textual diff has\n-an added or a deleted line that matches the given __. This\n-means that it will detect in-file (or what rename-detection considers\n-the same file) moves, which is noise. The implementation runs diff\n-twice and greps, and this can be quite expensive. To speed things up,\n-binary files without textconv filters will be ignored.\n-\n-When `-S` or `-G` are used without `--pickaxe-all`, only filepairs\n-that match their respective criterion are kept in the output. When\n-`--pickaxe-all` is used, if even one filepair matches their respective\n-criterion in a changeset, the entire changeset is kept. This behavior\n-is designed to make reviewing changes in the context of the whole\n-changeset easier.\n+way. `--patch-modifies=` and `--patch-grep=` are used\n+to specify different ways these strings are sought.\n+\n+`--patch-modifies=` (`-S` for short) detects filepairs\n+whose preimage and postimage have different number of occurrences of\n+the specified __. By definition, it will not detect in-file\n+moves. Also, when a changeset moves a file wholesale without\n+affecting the interesting string, diffcore-rename kicks in as usual,\n+and `--patch-modifies` omits the filepair (since the number of\n+occurrences of that string didn't change in that rename-detected\n+filepair). When used with `--pickaxe-regex`, treat the __ as\n+an extended POSIX regular expression to match, instead of a literal\n+string.\n+\n+`--patch-grep=` (`-G` for short, mnemonic: grep) detects\n+filepairs whose textual diff has an added or a deleted line that\n+matches the given regular expression. This means that it will detect\n+in-file (or what rename-detection considers the same file) moves,\n+which is noise. The implementation runs diff twice and greps, and\n+this can be quite expensive. To speed things up, binary files without\n+textconv filters will be ignored.\n+\n+When `--patch-modifies` or `--patch-grep` are used without\n+`--pickaxe-all`, only filepairs that match their respective criterion\n+are kept in the output. When `--pickaxe-all` is used, if even one\n+filepair matches their respective criterion in a changeset, the entire\n+changeset is kept. This behavior is designed to make reviewing\n+changes in the context of the whole changeset easier.\n \n diffcore-order: For Sorting the Output Based on Filenames\n ---------------------------------------------------------\n-- \n2.45.2\n\n"},{"id":"512281","messageId":"ceab16cd-1b26-453c-a326-d76f16b35814@gmail.com","threadId":"62909","inReplyTo":"xmqqwmdwxsmz.fsf@gitster.g","subject":"Re: [PATCH v4 0/10] Long names for `git log -S` and `git log -G`","fromName":"Illia Bobyr","fromEmail":"illia.bobyr@gmail.com","sentAt":"2025-02-12T03:28:49Z","receivedAt":"2025-02-12T03:28:50Z","isPatch":true,"sender":{"key":"illia.bobyr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/694419?v=4"},"body":"On 2/11/25 10:07, Junio C Hamano wrote:\n > Illia Bobyr writes:\n >\n >> I've split the big change from v3 [1] into multiple, mostly \nindependent patches\n >> to make it easier to review and merge each one separately.\n >>\n >> [1] \nhttps://lore.kernel.org/git/20250206014324.1839232-1-illia.bobyr@gmail.com/\n >>\n >> Patches 1 through 4 are fixing minor bugs and inconsistencies.\n >>\n >> Patch 5 contains updates gitdiffcore to use same placeholder names \nas the rest\n >> of the code.\n >>\n >> Patch 6 contains a minimum change to add long versions of -S and -G.\n >>\n >> Patch 7 adds bash completion support.\n >>\n >> Patches 8 through 10 increase usage of the long argument versions in \ntests, CLI\n >> help and docs respectively.\n >>\n >> Please, let me know if you prefer it split in a different way, or \nreorder the\n >> changes.\n >\n > When you base your patch on a different base than 'master' (or if\n > the previous iteration of the topic has already been queued in my\n > tree, then the commit used as the base to queue the topic), please\n > make sure you state it clearly.\n >\n > This iteration seems to apply on none of bc204b74 (The seventh\n > batch, 2025-02-03), on top of which the previous round dcc02caba2\n > (ib/diff-S-G-with-longhand) has been queued, or any of the recent\n > tips of 'master', like 388218fa (The ninth batch, 2025-02-10) or\n > 9520f7d9 (The eighth batch, 2025-02-06), so I cannot look at it.\n\nSorry for the confusion.  I randomly decided to check if my changes have any\nconflicts with `next` and rebased on top of it.\nDid not realize it would affect the patches.\nI've rebased back on top of `master` and published as v5.\n\n >> I was not sure if I should include a reference to the previous \nversion of the\n >> patch into the next reroll.  It seems that\n >> `Documentation/MyFirstContribution.adoc` suggests so. But it creates \nvery long\n >> threads.  And I've noticed that not everyone is doing it.\n >\n > Almost everybody does so, actually.\n >\n > Taking a topic that has 5 iterations, each about ~20 patches, as an\n > example:\n >\n > \nhttps://lore.kernel.org/git/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im/\n >\n > it is perfectly clear and easy to nagivate from the list of messages\n > what discussions we had in previous iterations.\n\nThank you for the explanation and for sharing an example link. I'll use v3\ncover letter as a reference point for v5, as I have already interrupted the\nreference chain in v3.\n\n >> Reply to review notes ...\n >\n > It is more customary to Reply-all directly to review messages,\n > instead of sending new round of patches.  When the cover letter of a\n > new iteration is sent as a response to the cover letter of the\n > previous iteration, readers can find the previous discussion\n > messages.\n\nGot it.  Thank you.  I have replied to your review email, so that we can\ncontinue the conversation there.\n"},{"id":"512319","messageId":"xmqq8qqbum49.fsf@gitster.g","threadId":"62909","inReplyTo":"081272b5-b035-47a6-a951-eb923a9a5833@gmail.com","subject":"Re: [PATCH v3 1/1] diff: --patch-{modifies,grep} arg names for -S and -G","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-12T17:08:22Z","receivedAt":"2025-02-12T17:08:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> My thinking is that as long version names improve readability, it also\n> applies\n> to the test code.  When I see a short option, I often have to check\n> the manual\n> to remember what exactly does it do.\n\nBut by now due to enough exposure, you have committed them in your\nmemory, no? ;-)\n\n> But, I think, I understand your point of view as well.\n\nYup, if the options were introduced with long and short forms at the\nsame time and the tests were written at the same time or shortly\nafter their introduction, I'd agree that using longer form more may\nbe beneficial, since there is nobody who is already familier with\neither of the forms. But at this point after 20 years, swapping one\nfor the other is mostly unnecessary churn, I would have to say (and\nI do not particularly want to having to repeat saying the same thing\nagain).\n\n>> OK.  NOte that this says .  We may want to have a separate\n> clean-up\n>> patch so that Documentation/gitdifcore.txt that used \n>> and the placeholder used here match.\n>\n> Makes sense.\n> I've added this fix as patch 5 in v5.\n\nI'd rather see these \"fixes to existing anomalies\" done totally\noutside of this series. IOW, I'd prefer to either (1) get the\nseries done with the minimally necessary changes first and then\nafter the dust settles from merging that to 'master', see these \"oh\nwe noticed these issues while working on the other series that has\nnow completed\" issues addressed, or (2) do the clean-up of existing\nanomalies first as a separate series, and then after the dust\nsettles for the clean-up, do the proposed addition of longform as a\nseparate series. I have slight preference to (1), simply because\nnobody complained on these small anomalies for the past 20 years ;-)\nbut I can also go with \"preliminary clean-up first\" route.\n\n>> This is an unrelated change that should not be in this patch.  If\n>> you want to modify it, please do it in a separate clean-up patch,\n>> just like the above vs change.\n>\n> Split it into patch 2 in v5.\n\nAgain, when I said \"unrelated\", I meant that I want them to be\ntreated as unrelated changes, addressed outside of this series,\neither in a preliminary clean-up, or after-the-dust-settles\nclean-up.\n\nThanks.\n"},{"id":"512338","messageId":"xmqqbjv6qyiv.fsf@gitster.g","threadId":"62909","inReplyTo":"20250212032657.1807939-2-illia.bobyr@gmail.com","subject":"Re: [PATCH v5 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-13T04:06:16Z","receivedAt":"2025-02-13T04:06:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> Subject: Re: [PATCH v5 01/10] t/t4209-log-pickaxe: Naming typo: -G takes a regex\n\n\"Naming\" -> \"naming\". It is even more preferable if you can avoid\nintroducing the second colon. E.g.\n\n Subject: t/t4209: call the value given to -G , not \n\nThis applies to the titles of all the patches in the series, I suspect?\n\n> Not effect on the test logic, but as \"-G\" argument is a regex it is more\n> accurate to use \"regex\" as a dummy argument value rather than \"string\".\n> In all the other case when \"-G\" is passed a dummy value it is spelled as\n> \"regex\" rather than as \"string\".\n\nI guess the -G tests are copied-and-pasted from existing tests for\nthe -S option when the -G option was introduced much later. This\nmakes me wonder if we try to see what happens when a malformed\nregular expression is fed to the -G option (I didn't check---if we\nhave no test for it, we might want to add one).\n\n> ---\n\nMissing sign-off.\n\nHaving said all that, I'd prefer to see the changes in one topic\nfocused only to give --patch-grep and --patch-modifies synonyms\nto the existing option and do nothing else. Certainly a change\nlike this is a distraction we do not have to discuss at the same\ntime.\n\nIf you can group all these \"preliminary clean-up\" changes together\ninto a separate series, without including any change to add the\nlonghand to -S/-G, that is also a viable alternative approach. Once\nsuch a series graduates to 'master', then you'd do the longhand on\ntop.\n\nWhat we do not want is to see changes that are not directly\nnecessary to add the longhand intermixed in the same series.\n\n> t/t4209-log-pickaxe.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t4209-log-pickaxe.sh b/t/t4209-log-pickaxe.sh\n> index a675ac..ed70c 100755\n> --- a/t/t4209-log-pickaxe.sh\n> +++ b/t/t4209-log-pickaxe.sh\n> @@ -89,7 +89,7 @@ test_expect_success 'usage: --no-pickaxe-regex' '\n> \ttest_expect_code 128 git log -Sstring --no-pickaxe-regex 2>actual &&\n> \ttest_cmp expect actual &&\n> \n> -\ttest_expect_code 128 git log -Gstring --no-pickaxe-regex 2>err &&\n> +\ttest_expect_code 128 git log -Gregex --no-pickaxe-regex 2>err &&\n> \ttest_cmp expect actual\n> '\n\nThanks.\n"},{"id":"512339","messageId":"xmqq5xleqy27.fsf@gitster.g","threadId":"62909","inReplyTo":"20250212032657.1807939-3-illia.bobyr@gmail.com","subject":"Re: [PATCH v5 02/10] diff: -G description: Correct copy/paste error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-13T04:16:16Z","receivedAt":"2025-02-13T04:16:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> Current description for -G is incorrect, seems like it was copied from\n> the description for -S.\n> ---\n> diff.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\n[jc: I won't point out about the title and sign-off, which are the\nsame issues [1/10] had and maybe shared with the later patches]\n\n>\n> diff --git a/diff.c b/diff.c\n> index 019fb..bd9db 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -5866,7 +5866,7 @@ struct option *add_diff_options(const struct option *opts,\n> \t\t\t N_(\"look for differences that change the number of occurrences of the specified string\"),\n> \t\t\t 0, diff_opt_pickaxe_string),\n> \t\tOPT_CALLBACK_F('G', NULL, options, N_(\"\"),\n> -\t\t\t N_(\"look for differences that change the number of occurrences of the specified regex\"),\n> +\t\t\t N_(\"look for differences where a patch contains the specified regex\"),\n\nYeah, but the updated one is not all that great, either. -S looks\nfor string, so either \"occurences\" or \"contains\" would work, but\na patch that \"contains\" the regular expression would not necessarily\nmatch with -G ;-) \n\n\"a patch contains a line that matches\" is closer but not correct.\n\nWhat the option looks for is if there is a changed line in the patch\nthat matches the given regular expression. If a context line shared\nbetween the preimage and the postimage matches the regular expression\nthat does not count as a \"hit\".\n\n N_(\"find differences with changed lines that match the given regex\").\n\nperhaps?\n\nAgain, this is \"preliminary clean-up\" (or \"after-the-dust-settles\")\nmaterial and shouldn't be part of the main series.\n\nThanks.\n"},{"id":"512340","messageId":"xmqqzfiqpj13.fsf@gitster.g","threadId":"62909","inReplyTo":"20250212032657.1807939-4-illia.bobyr@gmail.com","subject":"Re: [PATCH v5 03/10] diff: short help: Correct -S description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-13T04:26:16Z","receivedAt":"2025-02-13T04:26:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> `-S` shows changes that modify the number of occurrences of the\n> specified string, rather than only those that either completely remove\n> it or add it for the first time.\n> ---\n\n[jc: title, sign-off, and this should be done outside the main topic\nare shared with other patches, so I won't repeat them]\n\n> diff.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/diff.h b/diff.h\n> index 0a566f..49ece3 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -606,7 +606,7 @@ void diffcore_fix_diff_index(void);\n> \" try unchanged files as candidate for copy detection.\\n\" \\\n> \" -l limit rename attempts up to paths.\\n\" \\\n> \" -O reorder diffs according to the .\\n\" \\\n> -\" -S find filepair whose only one side contains the string.\\n\" \\\n> +\" -S find filepair who differ in the number of occurrences of string.\\n\" \\\n\nGiven that the initial motivation of -S was to find\n\"before it wasn't there in that shape, now there it is\" [*1*] (and\n wasn meant to be something unique in the codebase),\nthe original conveys the intent better, but the updated text\ndescribes the actual behaviour more correctly (in other words, the\nbehaviour of the option when a not-so-unique string that may produce\nmultiple hits is given is whatever it happens to do---which is \"see\nthe number of occurrences are different\").\n\nSo this is a good change.\n\n\n[Foornote]\n\n *1* The original motivation of \"-S\" was to be a\n building block for the tool Linus dreamed in his message\n https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org\n that explained why recording renames at the commit time is a\n bad idea.\n\n\n> \" --pickaxe-all\\n\" \\\n> \" show all files diff when -S is used and hit is found.\\n\" \\\n> \" -a --text treat all files as text.\\n\"\n"},{"id":"512341","messageId":"xmqqtt8ypikf.fsf@gitster.g","threadId":"62909","inReplyTo":"20250212032657.1807939-6-illia.bobyr@gmail.com","subject":"Re: [PATCH v5 05/10] docs: gitdiffcore: -G and -S: Use regex/string placeholders","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-13T04:36:16Z","receivedAt":"2025-02-13T04:36:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> In the rest of the documentation (and in the code) we use `regex` and\n> `string` as `-G` and `-S` argument placeholders. While\n> `regular-expression` and `block-of-text` are a bit easier to read, it is\n> a bit consistent.\n>\n> And we could assume that everyone who uses git should be able to\n> understand that a \"string\" and a \"block-of-text\", as well as a \"regex\"\n> and \"regular-expression\" are the same thing. So, using a shorter\n> version is also more consistent.\n> ---\n> Documentation/gitdiffcore.txt | 23 +++++++++++------------\n> 1 file changed, 11 insertions(+), 12 deletions(-)\n\nI am with one reservation. was written to stress\nthe fact that most of the time the string is expected to be a\nmulti-line block of text (like a function body) [*1*] that is unique\nwithin the codebase. I do not think replacing a short-and-sweet\n with is a good idea, but if we are to go\nthis route, we should mention that to compensate for the diminished\nstress on that block-ness of the text, as would imply\nsomething quite short and would fit on a single line.\n\n[Footnote]\n\n*1* Go back to Linus's message I cited earlier;\n https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org/\n"},{"id":"512342","messageId":"xmqqmseqpgjg.fsf@gitster.g","threadId":"62909","inReplyTo":"20250212032657.1807939-7-illia.bobyr@gmail.com","subject":"Re: [PATCH v5 06/10] diff: --patch-{grep,modifies} arg names for -G and -S","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-13T05:20:03Z","receivedAt":"2025-02-13T05:20:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> Most arguments have both short and long versions. Long versions are\n> easier to read, especially in scripts and command history.\n>\n> This change mostly keeps existing uses of -G and -S as is in the tests,\n> documentation and help output.\n>\n> Tests that check just the option parsing are duplicated to check both\n> short and long argument options.\n>\n> Signed-off-by: Illia Bobyr \n> ---\n\nThis step looks mostly good, but the option descriptions for\nexisting \"-S\" and \"-G\" have been touched by clean-up changes\nearlier, so they cannot be separated out.\n\nWe can treat [01-05/10] as a separate 5-patch \"preliminary\nclean-up\" series and discard this and later steps until the earlier\nhalf lands.\n\nUnlike other earlier steps, this one has a decent title and it has\nyour sign-off, both of which are good. And of course it is very\nmuch on topic.\n"},{"id":"512344","messageId":"xmqq8qqapfbr.fsf@gitster.g","threadId":"62909","inReplyTo":"20250212032657.1807939-8-illia.bobyr@gmail.com","subject":"Re: [PATCH v5 07/10] completion: Support --patch-{grep,modifies}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-13T05:46:16Z","receivedAt":"2025-02-13T05:46:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Illia Bobyr writes:\n\n> Subject: Re: [PATCH v5 07/10] completion: Support --patch-{grep,modifies}\n\n\"Support\" -> \"support\".\n\n\n> ---\n> contrib/completion/git-completion.bash | 11 ++++++++++-\n> 1 file changed, 10 insertions(+), 1 deletion(-)\n\nMissing sign-off.\n\nOther than that, this is a great material to be part of the main\ntopic to add the longhands to these two options.\n\nThanks.\n\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 41391..daf335 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1900,6 +1900,7 @@ __git_diff_common_options=\"--stat --numstat --shortstat --summary\n> \t\t\t--output= --output-indicator-context=\n> \t\t\t--output-indicator-new= --output-indicator-old=\n> \t\t\t--ws-error-highlight=\n> +\t\t\t--patch-grep= --patch-modifies=\n> \t\t\t--pickaxe-all --pickaxe-regex --patch-with-raw\n> \"\n> \n> @@ -2216,7 +2217,7 @@ __git_complete_log_opts ()\n> \t\t__git_complete_symbol --cur=\"${cur#:}\" --sfx=\":\"\n> \t\treturn\n> \t\t;;\n> -\t-G,*|-S,*)\n> +\t-G,*|--patch-grep,*|-S,*|--patch-modifies,*)\n> \t\t__git_complete_symbol\n> \t\treturn\n> \t\t;;\n> @@ -2239,6 +2240,14 @@ __git_complete_log_opts ()\n> \t\t__gitcomp \"$__git_diff_algorithms\" \"\" \"${cur##--diff-algorithm=}\"\n> \t\treturn\n> \t\t;;\n> +\t--patch-grep=*)\n> +\t\t__git_complete_symbol --pfx=\"--patch-grep=\" --cur=\"${cur#--patch-grep=}\"\n> +\t\treturn\n> +\t\t;;\n> +\t--patch-modifies=*)\n> +\t\t__git_complete_symbol --pfx=\"--patch-modifies=\" --cur=\"${cur#--patch-modifies=}\"\n> +\t\treturn\n> +\t\t;;\n> \t--submodule=*)\n> \t\t__gitcomp \"$__git_diff_submodule_formats\" \"\" \"${cur##--submodule=}\"\n> \t\treturn\n"}]}