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

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

[Martin Langhoff]

git log -G'regex' is a very usable alternative to the classic
pickaxe. Minimal patch to make it usable from gitk.
---
 gitk-git/gitk |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 22270ce..876b8f9 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 "changes match 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 "changes match regex:"]} {
+	set gdtargs [list "-G$highlight_files"]
     } else {
 	# must be "containing:", i.e. we're searching commit info
 	return
-- 
1.7.7.6

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

[Zbigniew Jędrzejewski-Szmek]

On 06/08/2012 04:32 AM, Martin Langhoff wrote:
> git log -G'regex' is a very usable alternative to the classic
> pickaxe. Minimal patch to make it usable from gitk.

>      set gm [makedroplist .tf.lbar.gdttype gdttype \
>  		[mc "containing:"] \
>  		[mc "touching paths:"] \
> -		[mc "adding/removing string:"]]
> +		[mc "adding/removing string:"] \
> +		[mc "changes match regex:"]]

Hi,

this feature is definitely useful. One nitpick: the string that you add
becomes part of a sentence in the GUI: "prev/next commit changes match
regex", which is in different grammatical form then the existing
snippets. Should be something with '-ing'.

Zbyszek

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

[Junio C Hamano]

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:

> On 06/08/2012 04:32 AM, Martin Langhoff wrote:
>> git log -G'regex' is a very usable alternative to the classic
>> pickaxe. Minimal patch to make it usable from gitk.
>
>>      set gm [makedroplist .tf.lbar.gdttype gdttype \
>>  		[mc "containing:"] \
>>  		[mc "touching paths:"] \
>> -		[mc "adding/removing string:"]]
>> +		[mc "adding/removing string:"] \
>> +		[mc "changes match regex:"]]
>
> Hi,
>
> this feature is definitely useful. One nitpick: the string that you add
> becomes part of a sentence in the GUI: "prev/next commit changes match
> regex", which is in different grammatical form then the existing
> snippets. Should be something with '-ing'.

"prev/next commit with changes matching regex" perhaps?

In any case, Paul needs to be in the loop when we discuss gitk
changes.

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

[Paul Mackerras]

On Sat, Jun 09, 2012 at 11:37:58PM -0700, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> 
> > On 06/08/2012 04:32 AM, Martin Langhoff wrote:
> >> git log -G'regex' is a very usable alternative to the classic
> >> pickaxe. Minimal patch to make it usable from gitk.
> >
> >>      set gm [makedroplist .tf.lbar.gdttype gdttype \
> >>  		[mc "containing:"] \
> >>  		[mc "touching paths:"] \
> >> -		[mc "adding/removing string:"]]
> >> +		[mc "adding/removing string:"] \
> >> +		[mc "changes match regex:"]]
> >
> > Hi,
> >
> > this feature is definitely useful. One nitpick: the string that you add
> > becomes part of a sentence in the GUI: "prev/next commit changes match
> > regex", which is in different grammatical form then the existing
> > snippets. Should be something with '-ing'.
> 
> "prev/next commit with changes matching regex" perhaps?

There is already the drop-down list to select exact, case-ignoring or
regexp matching.  I would think that "adding/removing string" combined
with "Regexp" matching should select this.

What exactly does git log -G do?

Paul.

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

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> 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'.

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

[Zbigniew Jędrzejewski-Szmek]

From: Martin Langhoff <martin@laptop.org>

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 <zbyszek@in.waw.pl>
---
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

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

[Martin Langhoff]

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
<zbyszek@in.waw.pl> wrote:
> From: Martin Langhoff <martin@laptop.org>
>
> 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 <zbyszek@in.waw.pl>
> ---
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
 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

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

[Felipe Contreras]

On Tue, May 7, 2013 at 12:17 PM, Martin Langhoff
<martin.langhoff@gmail.com> 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

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

[Paul Mackerras]

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.

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

[Jonathan Nieder]

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

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

[Paul Mackerras]

On Fri, May 10, 2013 at 11:13:22PM -0700, Jonathan Nieder wrote:
> 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.  

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.

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

[Martin Langhoff]

On Sat, May 11, 2013 at 5:41 AM, Paul Mackerras <paulus@samba.org> wrote:
> On Fri, May 10, 2013 at 11:13:22PM -0700, Jonathan Nieder wrote:
>> 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.
>
> 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

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

[Jonathan Nieder]

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

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

[Martin Langhoff]

On Mon, May 13, 2013 at 2:55 PM, Jonathan Nieder <jrnieder@gmail.com> 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

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

[Jonathan Nieder]

Martin Langhoff wrote:
> On Mon, May 13, 2013 at 2:55 PM, Jonathan Nieder <jrnieder@gmail.com> 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.

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.

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

[Martin Langhoff]

On Mon, May 13, 2013 at 3:33 PM, Jonathan Nieder <jrnieder@gmail.com> 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

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

[Junio C Hamano]

Martin Langhoff <martin.langhoff@gmail.com> writes:

> On Mon, May 13, 2013 at 3:33 PM, Jonathan Nieder <jrnieder@gmail.com> 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,

Since its beginning, the -S implementation hasn't change that much,
and I do not remember fixing such a bug. If you saw issues in old
Git, the same issues would still exist in today's Git.

It could be that a change to your history (not change to Git) was
introduced in an evil merge, and you were running "git log -p -S"
without "-m", or something.

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

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> Martin Langhoff wrote:
>> On Mon, May 13, 2013 at 2:55 PM, Jonathan Nieder <jrnieder@gmail.com> 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.
>
> 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.

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

[Jonathan Nieder]

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

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

[Jonathan Nieder]

Paul Mackerras wrote:

> How about "changing lines matching:"?

Sorry for the slow response.  Sounds perfect.

Thanks,
Jonathan