gitk highlight feature

gitk highlight feature

[Paul Mackerras]

I just pushed out some changes to gitk which allow you to use one view
to highlight another (see the "Highlight" submenu under the "View"
menu), and which allow you to specify arbitrary git-rev-list arguments
for a view.  The arguments string uses shell quoting conventions.

I had been thinking of having fields in the view editor dialog where
you could put in refs that you did and didn't want included, date
specifiers, etc., all in separate fields with suitable labels.  Now
I'm thinking that it's probably just as convenient to put
"ORIG_HEAD.." into the git-rev-list arguments field as it is to put
"ORIG_HEAD" in the "Don't include commits reachable from this" field.
There may be an argument for having fields for "Exclude commits before
this date" and "Exclude commits after this date", because those things
often have spaces in them (e.g. "2 weeks ago") which would have to be
quoted in the git-rev-list arguments field.

Thoughts?

Paul.

Re: gitk highlight feature

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

> I just pushed out some changes to gitk which allow you to use one view
> to highlight another (see the "Highlight" submenu under the "View"
> menu), and which allow you to specify arbitrary git-rev-list arguments
> for a view.  The arguments string uses shell quoting conventions.

Eh, the string entered by me is quoted by the program, or do I
have to quote it myself?  I suspect it should not be so bad to
code, even if you have to do it with tcl ;-).

> I had been thinking of having fields in the view editor dialog where
> you could put in refs that you did and didn't want included, date
> specifiers, etc., all in separate fields with suitable labels.  Now
> I'm thinking that it's probably just as convenient to put
> "ORIG_HEAD.." into the git-rev-list arguments field as it is to put
> "ORIG_HEAD" in the "Don't include commits reachable from this" field.
> There may be an argument for having fields for "Exclude commits before
> this date" and "Exclude commits after this date", because those things
> often have spaces in them (e.g. "2 weeks ago") which would have to be
> quoted in the git-rev-list arguments field.
>
> Thoughts?

Calendar widgets.  BTW, "rev-list --since=2.days.ago" would work
rather well ;-).

Re: gitk highlight feature

[Linus Torvalds]

On Wed, 3 May 2006, Paul Mackerras wrote:
>
> I just pushed out some changes to gitk which allow you to use one view
> to highlight another (see the "Highlight" submenu under the "View"
> menu), and which allow you to specify arbitrary git-rev-list arguments
> for a view.  The arguments string uses shell quoting conventions.

Ok. It looks interesting. I don't have a particular issue to try it with, 
but the bold-face certainly stands out enough that it was easy to ask for 
a high-light of any commits that changed the kernel/ subdirectory, and it 
was visually very obvious.

The interface does feel a bit awkward, though. Separating out "view" and 
"highlight" into two separate things seems wrong. Wouldn't it be better to 
just have multiple views in the "view" menu, and just a way to mark one or 
more of them as "highliht views". 

Ie not a separate menu, but having the current "view" radio buttons be 
more flexible. 

> I had been thinking of having fields in the view editor dialog where
> you could put in refs that you did and didn't want included, date
> specifiers, etc., all in separate fields with suitable labels.  Now
> I'm thinking that it's probably just as convenient to put
> "ORIG_HEAD.." into the git-rev-list arguments field as it is to put
> "ORIG_HEAD" in the "Don't include commits reachable from this" field.

Yeah. I think it's easier with a single thing, just let people stick it 
there.

> There may be an argument for having fields for "Exclude commits before
> this date" and "Exclude commits after this date", because those things
> often have spaces in them (e.g. "2 weeks ago") which would have to be
> quoted in the git-rev-list arguments field.

I alwaus use "2.weeks.ago" instead, but I guess you could do a calendar 
widget or something.

		Linus

Re: gitk highlight feature

[Paul Mackerras]

Junio C Hamano writes:

> Eh, the string entered by me is quoted by the program, or do I
> have to quote it myself?  I suspect it should not be so bad to
> code, even if you have to do it with tcl ;-).

It's split up into arguments at whitespace unless the whitespace is
quoted, just like the shell does.  I wrote tcl code to do the
splitting and quoting according to the shell rules.  So you can, for
example, put:

	--unpacked --since='2 days ago'

in there and it will pass two arguments to git-rev-list, the second
containing the embedded spaces (but without the single-quotes).

Paul.

Re: gitk highlight feature

[Linus Torvalds]

On Tue, 2 May 2006, Linus Torvalds wrote:
> 
> Ie not a separate menu, but having the current "view" radio buttons be 
> more flexible. 

The obvious way to do it would be to just have two buttons per view: one 
exclusive one (for the main view - only one selected at a time), and the 
other one for the "highlight" one where you could allow multiple views to 
be selected for highlighting.

		Linus

Re: gitk highlight feature

[Paul Mackerras]

Linus Torvalds writes:

> The obvious way to do it would be to just have two buttons per view: one 
> exclusive one (for the main view - only one selected at a time), and the 
> other one for the "highlight" one where you could allow multiple views to 
> be selected for highlighting.

That's hard to do in a menu, but I could do it in a separate pane.
Or, I could draw a series of tabs between the menu bar and the
graph/headline/author/date panes.  Each tab would have the view name,
a radiobutton to select it for highlighting, and a close button.

Paul.

Re: gitk highlight feature

[Linus Torvalds]

On Wed, 3 May 2006, Paul Mackerras wrote:
>
> Linus Torvalds writes:
> 
> > The obvious way to do it would be to just have two buttons per view: one 
> > exclusive one (for the main view - only one selected at a time), and the 
> > other one for the "highlight" one where you could allow multiple views to 
> > be selected for highlighting.
> 
> That's hard to do in a menu, but I could do it in a separate pane.
> Or, I could draw a series of tabs between the menu bar and the
> graph/headline/author/date panes.  Each tab would have the view name,
> a radiobutton to select it for highlighting, and a close button.

Actually, having used this thing (and thought about) some more, I'm afraid 
that I'll have to just admit that my whole "highlight" thing was 
misdesigned.

One is just an interface issue, and I think that it would be fixed by just 
removing the highlight entry from the "view" menu, and making it a menu of 
its own. That, I think, would fix the user interface to be more obvious 
and intuitive.

But the real thing I found is that when I decided I wanted to highlight, I 
didn't actually want to highlight by "git-rev-list" at all. At least not 
most of the time.

So far, what I've wanted to highlight by is:

 - "does it touch this file/directory/pathspec"

   This is _close_ to "git-rev-list", and you can (and do) actually 
   implement it as that, but it's stupid to do it that way. You just spend 
   extra time. It's literally much better to do

	cat commit-list | git-diff-tree -s --stdin -- <pathspec>

   which is a hell of a lot more efficient, since you already have the 
   commit-list you're interested in (and, in fact, this allows you to do 
   things efficiently only for the current _visible_ commits, if you want 
   to, which might be an important optimization for large views).

 - "Does the author/committer match xyz*"

   I ended up using the "search" button for this, and it worked, but the 
   highlight feature would just have done it much better. Especially if 
   there was a way to do "go to next highlight", instead of just "go to 
   next commit"

   Again, this is actually most easily done by just using the commit-list 
   you already _have_. Not with adding a new argument to "git-rev-list" 
   and trying to filter two views. Also, again, this can actually be done 
   for just the "visible" ones, not the whole view.

 - "highlight the commits [not] reachable from the selected commit"

   Now, this literally is "git-rev-list", but it's a variation on it: you 
   don't want a new view, you really want the _old_ view (exact same 
   git-rev-list arguments), but you add "^commit" to the list of 
   revisions. And then you (conditionally) invert the highlighting if you 
   wanted "reachable" rather than "not reachable".

And the current "create a new view, and overlay that on the old one" 
approach really isn't that good. My mistake, it was me who suggested it.

What do you think? 

		Linus

Re: gitk highlight feature

[Paul Mackerras]

Linus Torvalds writes:

> So far, what I've wanted to highlight by is:
> 
>  - "does it touch this file/directory/pathspec"
> 
>    This is _close_ to "git-rev-list", and you can (and do) actually 
>    implement it as that, but it's stupid to do it that way. You just spend 
>    extra time. It's literally much better to do
> 
> 	cat commit-list | git-diff-tree -s --stdin -- <pathspec>
> 
>    which is a hell of a lot more efficient, since you already have the 

On my G5:

$ git-rev-list HEAD >all-revs
$ time git-rev-list HEAD -- arch/powerpc >ppc-revs
real    0m2.463s
user    0m2.265s
sys     0m0.191s
$ time git-diff-tree -s --stdin -- arch/powerpc <all-revs >ppc-revs2
real    0m5.269s
user    0m4.794s
sys     0m0.462s
$ wc ppc-revs*
   892    892  36572 ppc-revs
  5875   5875 240875 ppc-revs2

Why does git-diff-tree -s --stdin produce so many more revisions than
git-rev-list?  The git-diff-tree output includes commits such as
6ba815de, which only affects arch/i386/kernel/timers/timer_tsc.c.
Confused.

>    commit-list you're interested in (and, in fact, this allows you to do 
>    things efficiently only for the current _visible_ commits, if you want 
>    to, which might be an important optimization for large views).

I'd have to try it.  The overhead of fork/exec/process startup might
be too much to do every time the user scrolls the window, though.
Maybe what we need is some way to have git-diff-tree run as a helper
process where I can write some commits to it, then write something
that says "that's all for now" and have it finish what it's doing,
write some "end of output" indicator and flush its output buffers -
and still have the process and the pipes to/from it available for
another round later.

>  - "Does the author/committer match xyz*"
> 
>    I ended up using the "search" button for this, and it worked, but the 
>    highlight feature would just have done it much better. Especially if 
>    there was a way to do "go to next highlight", instead of just "go to 
>    next commit"

The "/" and "?" keys go to the next and previous find hit.  It sounds
like the search function is actually what you want for this, since it
does highlight the commits that match, but maybe the problem is that
the yellow highlighting tends to go away too easily.

How about a way to do a search and use the results to create a
bold highlight, which will persist even if you do another search?
Either a separate "find & highlight" button, or a way to transfer the
current find hits to the highlight?

As for "go to next/previous highlight", that sounds useful.  Rusty
wants "go to next/previous commit that is a child/parent of this one"
and "go to next/previous commit that is a descendent/ancestor of this
one and has a ref (tag, head, etc.)".

How about shift-up and shift-down for "go to previous/next highlight"?

Paul.

Re: gitk highlight feature

[Linus Torvalds]

On Thu, 4 May 2006, Paul Mackerras wrote:
> 
> On my G5:
> 
> $ git-rev-list HEAD >all-revs
> $ time git-rev-list HEAD -- arch/powerpc >ppc-revs
> real    0m2.463s
> user    0m2.265s
> sys     0m0.191s
> $ time git-diff-tree -s --stdin -- arch/powerpc <all-revs >ppc-revs2
> real    0m5.269s
> user    0m4.794s
> sys     0m0.462s

Right. One of them prunes the history (git-rev-list). The other one does 
not.

That said, you need an "-r", I guess, to "git-diff-tree".

> Why does git-diff-tree -s --stdin produce so many more revisions than
> git-rev-list?

I guess it's officially a FAQ by now: see the "bug in git log" thread the 
other day, and

        http://www.gelato.unsw.edu.au/archives/git/0604/19180.html

so in general you can get _more_ of the "real commits" with "git-rev-list 
| git-diff-tree --stdin", because it won't prune out history on 
uninteresting branches. At the same time, that effect is counter-acted by 
git-diff-tree normally ignoring merges by default, which is a bigger 
issue.

Your big problem is just the lack of "-r", though:

> The git-diff-tree output includes commits such as
> 6ba815de, which only affects arch/i386/kernel/timers/timer_tsc.c.
> Confused.

Without the -r, git-diff-tree won't actually recurse, so it hits the 
"arch/" part (which does differ, and matches te revspec), and decides it's 
done.

With the "-r" thing, you still have a noticeable difference, but now it's 
due to the difference between "log" and "diff":

   git-rev-list HEAD -- arch/powerpc | wc -l
    -> 892
   git-rev-list HEAD | git-diff-tree -r -s --stdin -- arch/powerpc/ | wc -l
    -> 838

ie "diff" doesn't show merges nomally (with -m, you _will_ get merges, but 
you'll sometimes get them twice - once against each parent ;)

		Linus

Re: gitk highlight feature

[Paul Mackerras]

Linus Torvalds writes:

> But the real thing I found is that when I decided I wanted to highlight, I 
> didn't actually want to highlight by "git-rev-list" at all. At least not 
> most of the time.

I just pushed some changes to the "new" branch of the gitk.git
repository which change the way we do highlighting.  There is now a
row of controls across the middle of the window, just below the row
containing the sha1 ID, "Find" button, etc., which controls the
highlighting.  There are (currently) three ways to do highlighting: by
path, by view, and by author/committer.  The author/committer matching
is case-insensitive (since I'm clearly an insensitive sort of guy :)
and matches any of the given strings anywhere in the author and
committer fields.  The path and author/committer entry widgets take a
whitespace-delimited list of paths or names of interest, using shell
quoting rules, so you can put for example:

"david s. miller" benh

in there and you'll get commits from either davem or benh highlighted.

Do people think this is useful and on the right track interface-wise?

Paul.

Re: gitk highlight feature

[Linus Torvalds]

On Sat, 20 May 2006, Paul Mackerras wrote:
> 
> I just pushed some changes to the "new" branch of the gitk.git
> repository which change the way we do highlighting.  There is now a
> row of controls across the middle of the window, just below the row
> containing the sha1 ID, "Find" button, etc., which controls the
> highlighting.

Ok, this is much nicer, I think this ends up being much closer to what I 
really wanted, even if I didn't know quite how I wanted it.

I think the "Find" field should highlight things too. Right now there's no 
way to get highlighting for somebody having signed-off on a patch, for 
example, even though you can _search_ for it.

Also, right now it says "Author/committer", but it actually only triggers 
on author. Which may be the right thing to do (it's certainly what I'd 
normally want to see), but it doesn't match the description. 

		Linus

Re: gitk highlight feature

[Paul Mackerras]

Linus Torvalds writes:

> I think the "Find" field should highlight things too. Right now there's no 
> way to get highlighting for somebody having signed-off on a patch, for 
> example, even though you can _search_ for it.

Yes, I think the way to go is to extend the highlight feature to be
able to do everything that the "Find" function can do, and then I
think the "Find" function won't be needed any more.

> Also, right now it says "Author/committer", but it actually only triggers 
> on author. Which may be the right thing to do (it's certainly what I'd 
> normally want to see), but it doesn't match the description. 

If the author matches, it highlights both the headline and the
author.  If the committer matches, it highlights only the headline (as
it does for any other highlighting).  Try it with "torvalds" in the
author/committer field on commit a54c9d30 (compared to cb46c370,
say).  If that's confusing I can change it of course.

Paul.