On 11/18/24 19:52, Junio C Hamano wrote:
`--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
That is probably OK (even though "-G" is not exactly what the
pickaxe machinery wants to do; "--grep-in-patch" might be closer to
the intent).
Imagining, that I am starting from scratch for this functionality, I think I
would also consider "patch". Though, as we have 4 related argument names, I
wonder if using it as a prefix would create a more consistent UX.
Something like:
"--patch-grep" for "-G"
"--patch-modifies" for "-S"
"--patch-search-show-all"/"--patch-show-all" for "--pickaxe-all"
"--patch-search-regex"/"--patch-regex" for "--pickaxe-regex"
I'm not too sure about the later two, and also they already have long names.
But it seems quite easy to understand what this command would do:
git log --patch-grep sometext ...
Or this
git log --patch-modifies function_call --patch-search-regex ...
Not sure what would be a reasonably short alternative for `-S`.
`--pickaxe-occurance-change` seems too long, and might not be as clear.
`--pickaxe-occurance-count-change` is just way too long.
Giving a tool a meaningful name is an excellent idea. If the
meaningful name guides users to the right way to use the tool,
it would be ideal. Which means that to name it right, you'd need to
know what it exactly is for.
Glad we are on the same page here :)
I would really appreciate yours and Jeff input.
I wrote a simple patch to see how much work is it to add long names [1].
But I would change it based on whatever names are agreed upon.
[1]:
https://lore.kernel.org/git/20241119032755.3360365-1-illia.bobyr@xxxxxxxxx/
The -S feature was written to become one of the building blocks of
Linus's "clearly superior algorithm", described in [1]. Linus talks
about "where did this _line_ come from?", but the algorithm is more
generally about a block of code. The expected use case is for -S to
be fed sufficiently unique block of text so that we can efficiently
detect the transition of occurence count from 1 (because wee start
from sufficiently unique block of code) down to 0 (which is the
boundary in history where the block of code was first introduced in
its current form). It detects any occurence count change, but its
primary focus is to find a transition from 1 to 0 (when going
backwards in history). Its spirit is more about "finding where it
appeared in its current shape".
[Footnote]
*1* https://lore.kernel.org/git/Pine.LNX.4.58.0504150753440.7211@xxxxxxxxxxxxxxx/
This is pretty interesting. Thank you for pointing it out. I guess, it
means
that the "counting" in the porposed long name of "-S" arguments is more
of an
implementation detail than the actually intended functionality. Do you
think
there is a word that better reflects the intent here? "--patch-modifies" is
probably also not really hitting it 100%.
On 11/19/24 10:58, Jeff King wrote:
On Tue, Nov 19, 2024 at 12:52:50PM +0900, Junio C Hamano wrote:
`--pickaxe-grep` for `-G` seems like a reasonable alternative name for `-G`.
That is probably OK (even though "-G" is not exactly what the
pickaxe machinery wants to do; "--grep-in-patch" might be closer to
the intent).
FWIW, I like --grep-in-patch. Saying just "--pickaxe-grep" does not
highlight that it is about looking in the patch. I.e., it is not clear
from the name that is different from "-Sfoo --pickaxe-regex".
I agree. Do you think "--patch-grep" is a better name? Or do you think
that
the grep and the occurrence counting functionality should not share a common
prefix, as they are somewhat different in nature?
"git log" docs talk about both "-S" and "-G" as if they are pretty
close. There
is a note in "git diffcore" doc, "diff-pickaxe" section that says that
grep is
actually quite different. I wonder if this is important for the users
to think
about. I often use "-G" followed by "-S" or the other way around, just
to see
which view is better for my particular problem.
