Re: [PATCH] gitk: add support for -G'regex' pickaxe variant

Paul Mackerras [off-list ref] writes:

What exactly does git log -G do?

Unlike

    git log -S'this
    and
    that
    line'

which finds commits that changes the number of occurrences of the
given lines in the blob (typically 1 to 0 or 0 to 1),

    git log -G'frotz'

internally runs "git log -U0", and finds commits that adds or
removes a line that matches the given pattern 'frotz'.

From: Martin Langhoff <redacted>

git log -G'regex' is a very usable alternative to the classic
pickaxe. Minimal patch to make it usable from gitk.

[zj: reword message]
Signed-off-by: Zbigniew Jędrzejewski-Szmek <redacted>
---
Martin's off on holidays, so I'm sending v2 after rewording.

 gitk-git/gitk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 22270ce..24eaead 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk

@@ -2232,7 +2232,8 @@ proc makewindow {} {
     set gm [makedroplist .tf.lbar.gdttype gdttype \
 		[mc "containing:"] \
 		[mc "touching paths:"] \
-		[mc "adding/removing string:"]]
+		[mc "adding/removing string:"] \
+		[mc "with changes matching regex:"]]
     trace add variable gdttype write gdttype_change
     pack .tf.lbar.gdttype -side left -fill y
 

@@ -4595,6 +4596,8 @@ proc do_file_hl {serial} {
 	set gdtargs [concat -- $relative_paths]
     } elseif {$gdttype eq [mc "adding/removing string:"]} {
 	set gdtargs [list "-S$highlight_files"]
+    } elseif {$gdttype eq [mc "with changes matching regex:"]} {
+	set gdtargs [list "-G$highlight_files"]
     } else {
 	# must be "containing:", i.e. we're searching commit info
 	return

-- 
1.7.11.rc3.129.ga90bc7a.dirty

I just did git rebase origin/master for the umpteenth time, which
reminded me this nice patch is still pending.

ping?



m

On Thu, Jun 14, 2012 at 2:34 PM, Zbigniew Jędrzejewski-Szmek
[off-list ref] wrote:

-- 
 martin.langhoff@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff

On Tue, May 7, 2013 at 12:17 PM, Martin Langhoff
[off-list ref] wrote:

I just did git rebase origin/master for the umpteenth time, which
reminded me this nice patch is still pending.

ping?

For some reason getting patches into gitk takes a long long looong time.

-- 
Felipe Contreras

On Tue, May 07, 2013 at 01:17:18PM -0400, Martin Langhoff wrote:

I just did git rebase origin/master for the umpteenth time, which
reminded me this nice patch is still pending.

ping?

I thought I had replied to this patch; maybe I only thought about it.

Given that we already have a selector to choose between exact and
regexp matching, it seems more natural to use that rather than add a
new selector entry.  Arguably the "IgnCase" option should be disabled
when "adding/removing string" is selected.

Paul.

Paul Mackerras wrote:

I thought I had replied to this patch; maybe I only thought about it.

Given that we already have a selector to choose between exact and
regexp matching, it seems more natural to use that rather than add a
new selector entry.  Arguably the "IgnCase" option should be disabled
when "adding/removing string" is selected.

Thanks.  I think I disagree: "log -G" and "log -S" are different
operations, not variations on the same one.  

The description "Find next commit adding/removing string:" very
clearly conveys what "-S" means.  Maybe -G would be more clearly
described as "Find next commit changing line that matches regex:" or
"Find next commit changing line containing:"?

Jonathan

On Fri, May 10, 2013 at 11:13:22PM -0700, Jonathan Nieder wrote:

Paul Mackerras wrote:

quoted

I thought I had replied to this patch; maybe I only thought about it.

Given that we already have a selector to choose between exact and
regexp matching, it seems more natural to use that rather than add a
new selector entry.  Arguably the "IgnCase" option should be disabled
when "adding/removing string" is selected.

Thanks.  I think I disagree: "log -G" and "log -S" are different
operations, not variations on the same one.  

OK, fair enough, and I see there is in fact a --pickaxe-regex we
could use.

The description "Find next commit adding/removing string:" very
clearly conveys what "-S" means.  Maybe -G would be more clearly
described as "Find next commit changing line that matches regex:" or
"Find next commit changing line containing:"?

How about "changing lines matching:"?  If it gets too long it will
take up too much horizontal room.

Paul.

On Sat, May 11, 2013 at 5:41 AM, Paul Mackerras [off-list ref] wrote:

On Fri, May 10, 2013 at 11:13:22PM -0700, Jonathan Nieder wrote:

quoted

Paul Mackerras wrote:

quoted

I thought I had replied to this patch; maybe I only thought about it.

Given that we already have a selector to choose between exact and
regexp matching, it seems more natural to use that rather than add a
new selector entry.  Arguably the "IgnCase" option should be disabled
when "adding/removing string" is selected.

Thanks.  I think I disagree: "log -G" and "log -S" are different
operations, not variations on the same one.

OK, fair enough, and I see there is in fact a --pickaxe-regex we
could use.

And to be honest, log -G is so much more useful that I don't care a
s***t for log -S.

Essentially, to use log -S you need to know that the string the change
you are looking for changed all together. That's a high threshold of
knowledge to meet before you find something; you essentially have to
know what you are looking for pretty well!

In other words: You find a suspicious-looking line of code and you ask
"how did this horrid code come to be?", and the more horrendous the
code is, the more likely it is to be the accretion of a several
commits. In that case, which to me is the common case, log -S ain't
your friend at all.

cheers,




m

Paul Mackerras wrote:

How about "changing lines matching:"?

Sorry for the slow response.  Sounds perfect.

Thanks,
Jonathan

Martin Langhoff wrote:

And to be honest, log -G is so much more useful that I don't care a
s***t for log -S.

Fair enough. :)

[...]

In other words: You find a suspicious-looking line of code and you ask
"how did this horrid code come to be?", and the more horrendous the
code is, the more likely it is to be the accretion of a several
commits. In that case, which to me is the common case, log -S ain't
your friend at all.

My experience is the opposite.  I wonder "What did the author of this
nonsense comment mean?" or "What is the purpose of this strange
condition in this if () statement?".  Then "git log -S" finds the
culprit without showing extraneous unrelated changes (such as
reindenting).  It is like "git blame", but for arbitrary chunks of
code instead of single lines.  Then, just like with "git blame", at
times the next step is to blame the parent and repeat the process
using the earlier form of the code in question.

It is especially handy for confusing code that spans multiple lines.
(Unfortunately that is not as easy to try in gitk.)

As I mentioned before, log -G and log -S are fairly dissimilar
operations.

Thanks,
Jonathan

On Mon, May 13, 2013 at 2:55 PM, Jonathan Nieder [off-list ref] wrote:

My experience is the opposite.  I wonder "What did the author of this
nonsense comment mean?" or "What is the purpose of this strange
condition in this if () statement?".  Then "git log -S" finds the
culprit

Only if that if () statement looks that way from a single commit.
That's my point. If the line code bit you are looking at is the result
of several changes, your log -S will grind a while and find you
nothing.

cheers,



m
--
 martin@laptop.org
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

Martin Langhoff wrote:

On Mon, May 13, 2013 at 2:55 PM, Jonathan Nieder [off-list ref] wrote:

quoted

My experience is the opposite.  I wonder "What did the author of this
nonsense comment mean?" or "What is the purpose of this strange
condition in this if () statement?".  Then "git log -S" finds the
culprit

Only if that if () statement looks that way from a single commit.
That's my point. If the line code bit you are looking at is the result
of several changes, your log -S will grind a while and find you
nothing.

Well, no, it should find the final change that brought it into the
current form.  Just like "git blame".

Has it been finding zero results in some cases where the current code
matches the pattern?  That sounds like a bug.

On Mon, May 13, 2013 at 3:33 PM, Jonathan Nieder [off-list ref] wrote:

Well, no, it should find the final change that brought it into the
current form.  Just like "git blame".

Has it been finding zero results in some cases where the current code
matches the pattern?  That sounds like a bug.

Ummm, maybe. You are right, with current git it does work as I would
expect (usefully ;-) ).

I know I struggled quite a bit with log -S not finding stuff I thought
it should and that log -G did find, back a year ago.

Damn, I don't have a precise record of what git it was on, nor a good
repro example. Too long ago,



m
--
 martin.langhoff@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff

Jonathan Nieder wrote:

Martin Langhoff wrote:

quoted

On Mon, May 13, 2013 at 2:55 PM, Jonathan Nieder [off-list ref] wrote:

quoted

quoted

My experience is the opposite.  I wonder "What did the author of this
nonsense comment mean?" or "What is the purpose of this strange
condition in this if () statement?".  Then "git log -S" finds the
culprit

Only if that if () statement looks that way from a single commit.
That's my point. If the line code bit you are looking at is the result
of several changes, your log -S will grind a while and find you
nothing.

Well, no, it should find the final change that brought it into the
current form.  Just like "git blame".

I still don't know exactly what -G and -S do.  The documentation can
be improved, no?  A nice example would definitely help.

Thanks.

Ramkumar Ramachandra wrote:

I still don't know exactly what -G and -S do.

If you've been following recent gitk development (or this thread)
closely, you'll know that "git log -S" finds commits adding/removing a
string, while "git log -G" finds commits changing lines matching a
regex.

Examples for inclusion in the documentation welcome, certainly
(especially if they come in patch form).

A quick web search finds some references:

 http://article.gmane.org/gmane.comp.version-control.git/187364
 http://thread.gmane.org/gmane.comp.version-control.git/154822/focus=154823
 http://gitster.livejournal.com/35628.html
 http://thread.gmane.org/gmane.comp.lang.scala.internals/14219/focus=14220
 http://thread.gmane.org/gmane.linux.kernel/1370389/focus=1370478
 http://thread.gmane.org/gmane.comp.sysutils.autoconf.general/10682/focus=5691
 http://thread.gmane.org/gmane.network.nagios.plugins.devel/6164/focus=6175
 http://thread.gmane.org/gmane.comp.python.sympy/5830/focus=5836

Thanks and hope that helps,
Jonathan