Improved git-gui blame viewer

Improved git-gui blame viewer

[Shawn O. Pearce]

A long time ago Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> On Sun, 18 Mar 2007, Shawn O. Pearce wrote:
> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > > 
> > > Of course, the git gui blame colorization is clearly done by somebody who 
> > > is still actively popping LSD with both fists and didn't realize that the 
> > > 60's are long done, but that's another issue.
> > 
> > git-gui is open source.  I'd be happy to take a patch.  Or,
> > since that is horribly messy Tcl/Tk code, just a better color
> > suggestion. :-)
> 
> I would suggest:
> 
>  - some special color for "currently selected" (which defaults to being 
>    the first one coming out of the blame thing, of course). 
> 
>    I'd suggest "black text on pale green background", but that may be just 
>    me.
> 
>  - some *stable* graduated color for the rest. I don't think it 
>    necessarily needs to be "older" vs "newer", and in fact I'd suggest 
>    just two slightly different shades of gray for the background - just 
>    pick alternating shades for each blame entry that comes in (and leave 
>    un-blamed lines white).

I finally got the git-gui code to the point where cleaning up the
user interface was possible without sending myself to the nut house.

I tried out Linus' suggestions for coloring, and I like them.  Enough
that they are now sitting in my `pu` branch on repo.or.cz/git-gui.git.

There's also a whole slew of other improvements to the blame viewer,
like being able to dig through history by clicking on commit ids,
and tooltips when you mouse over a region of the file.

Behavior on Windows is actually quite good; its less so on Mac
OS X.  I'm fighting Tk there a little bit more than I should be.
Untested on Linux, so I'd love to hear some feedback on it.

  git://repo.or.cz/git-gui.git      pu
  http://repo.or.cz/r/git-gui.git   pu
 
-- 
Shawn.

Re: Improved git-gui blame viewer

[Matthijs Melchior]

Shawn O. Pearce wrote:
> A long time ago Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> On Sun, 18 Mar 2007, Shawn O. Pearce wrote:
>>> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>>> Of course, the git gui blame colorization is clearly done by somebody who 
>>>> is still actively popping LSD with both fists and didn't realize that the 
>>>> 60's are long done, but that's another issue.
>>> git-gui is open source.  I'd be happy to take a patch.  Or,
>>> since that is horribly messy Tcl/Tk code, just a better color
>>> suggestion. :-)
>> I would suggest:
>>
>>  - some special color for "currently selected" (which defaults to being 
>>    the first one coming out of the blame thing, of course). 
>>
>>    I'd suggest "black text on pale green background", but that may be just 
>>    me.
>>
>>  - some *stable* graduated color for the rest. I don't think it 
>>    necessarily needs to be "older" vs "newer", and in fact I'd suggest 
>>    just two slightly different shades of gray for the background - just 
>>    pick alternating shades for each blame entry that comes in (and leave 
>>    un-blamed lines white).
> 
> I finally got the git-gui code to the point where cleaning up the
> user interface was possible without sending myself to the nut house.
> 
> I tried out Linus' suggestions for coloring, and I like them.  Enough
> that they are now sitting in my `pu` branch on repo.or.cz/git-gui.git.
> 
> There's also a whole slew of other improvements to the blame viewer,
> like being able to dig through history by clicking on commit ids,
> and tooltips when you mouse over a region of the file.
> 
> Behavior on Windows is actually quite good; its less so on Mac
> OS X.  I'm fighting Tk there a little bit more than I should be.
> Untested on Linux, so I'd love to hear some feedback on it.
> 
>   git://repo.or.cz/git-gui.git      pu
>   http://repo.or.cz/r/git-gui.git   pu
>  

I have got this on Debian Etch and given the command 'make install'
in the pu branch while git-core 1.5.2 is installed.

About git-gui says:
	git-gui version 0.7.2.39.ge6a1
	git version 1.5.2
	Tcl/Tk version 8.4.12

The colors look much better.

The behavior has some rough edges. I don't like the following:
  When clicking on a link in the left column, the file as present in
  that commit is loaded, positioned at the top. I would like for the
  line where I clicked is to stay at the same position on the screen,
  so I do not have to find it again.

  Also, when returning I would like most lines on the screen stay the
  same.

  When clicking on a light gray line to become a green line, then
  adjacent areas are not correctly colored.  A few adjacent entries
  become all same gray... [Look around git-gui.sh:340]


  Something I want for the normal window, in the Staged and Unstaged
  file lists, high-lite the last entry selected so it becomes easy to
  click on the next one and I can see more clearly what is displayed
  in the bottom area.


Thanks for your efforts!

Regards,
	Matthijs Melchior.

Re: Improved git-gui blame viewer

[Shawn O. Pearce]

Matthijs Melchior <mmelchior@xs4all.nl> wrote:
> The colors look much better.

Thank Linus.  He's not a user inteface guy, but his idea was actually
pretty good.  ;-)
 
> The behavior has some rough edges. I don't like the following:
>   When clicking on a link in the left column, the file as present in
>   that commit is loaded, positioned at the top. I would like for the
>   line where I clicked is to stay at the same position on the screen,
>   so I do not have to find it again.

Me too.  Unfortunately I'm fighting with Tk to make this work.
Right now I've got the code written to do this, and I see it happen
internally, but then something causes Tk to reset the viewport back
to the top.  Arrrrrrgh.  I haven't pushed it out because it doesn't
work.
 
>   Also, when returning I would like most lines on the screen stay the
>   same.

Ditto.

>   When clicking on a light gray line to become a green line, then
>   adjacent areas are not correctly colored.  A few adjacent entries
>   become all same gray... [Look around git-gui.sh:340]

This (I think) is because of the way the color selections are
being done.  git-gui is being stupid and just alternating colors to
commits as they come in from `git blame --incremental`.  The thing
about the incremental blame is I can receive data for any part of
the file at any time.  So in general what happens is I get data for
one part of the file, give it color A, then data for another part,
give it color B, and then get data for part that is right next to the
first A and assign it A again.  So you see chunks where there is no
alternating...

>   Something I want for the normal window, in the Staged and Unstaged
>   file lists, high-lite the last entry selected so it becomes easy to
>   click on the next one and I can see more clearly what is displayed
>   in the bottom area.

I'm not sure I understand what you are looking for here.  Right now
git-gui should be inverting the foreground/background colors on
the file that is "selected" (shown in the lower diff view pane).
So the background should be black, and the foreground white.
Is this not happening?  Or are you looking for something else?

-- 
Shawn.

Re: Improved git-gui blame viewer

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]

hoi :)

On Mon, Jun 04, 2007 at 02:07:20AM -0400, Shawn O. Pearce wrote:
> >   When clicking on a light gray line to become a green line, then
> >   adjacent areas are not correctly colored.  A few adjacent entries
> >   become all same gray... [Look around git-gui.sh:340]
> 
> This (I think) is because of the way the color selections are
> being done.  git-gui is being stupid and just alternating colors to
> commits as they come in from `git blame --incremental`.  The thing
> about the incremental blame is I can receive data for any part of
> the file at any time.  So in general what happens is I get data for
> one part of the file, give it color A, then data for another part,
> give it color B, and then get data for part that is right next to the
> first A and assign it A again.  So you see chunks where there is no
> alternating...

If you use three colors you can always select one which is different
to the hunk above and below.  But I don't know if that would be
visually appealing...

Another nice thing would be a smooth gradient for each hunk.
Then we could use the same colors for every hunk, but the top of each
hunk would be a little bit lighter/darker than the bottom so that
it is easy to see the border.  Is that doable in Tk?
Perhaps a simple small line between hunks is enough, too?

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Improved git-gui blame viewer

[Shawn O. Pearce]

Martin Waitz <tali@admingilde.org> wrote:
> 
> On Mon, Jun 04, 2007 at 02:07:20AM -0400, Shawn O. Pearce wrote:
> > >   When clicking on a light gray line to become a green line, then
> > >   adjacent areas are not correctly colored.  A few adjacent entries
> > >   become all same gray... [Look around git-gui.sh:340]
> 
> If you use three colors you can always select one which is different
> to the hunk above and below.  But I don't know if that would be
> visually appealing...

That's actually not a bad idea.  But to make that work I have to
do the coloring by line chunks, not commits.  Given how bad the
current by-commit coloring is, the 3 coloring by line chunks is
probably the best bet.  It would resemble what gitweb does, but
we'd be using 3 colors in git-gui vs. the 2 in gitweb.  We could
do worse.
 
> Another nice thing would be a smooth gradient for each hunk.
> Then we could use the same colors for every hunk, but the top of each
> hunk would be a little bit lighter/darker than the bottom so that
> it is easy to see the border.  Is that doable in Tk?

I think so, but its ugly.  The viewer is actually 4 text widgets
crammed next to each other.  I can set the background color of a
line by giving it a tag, so to do a gradient I have to assign a
different background color to each line by giving each line its
own tag (ick).  Worse, in a 3 line chunk I can only do 3 colors.
That fails your "smooth" concept.  ;-)

> Perhaps a simple small line between hunks is enough, too?

That would be messy.  I can certainly cause a few pixels of spacing
to show up between chunks, but I'm reading the data "live" from the
blame engine and putting it on screen.  Adding space betwen chunks
as I get it will cause the data to "reflow" while you are trying to
read it.  I can probably account for it with the scrollbar and adjust
it accordingly, but at some point you will wind up seeing the text
in the viewer pane moving around and expanding as the padding gets
tossed in.


BTW, I just got the jump-to-original line and restore-view-on-back
features that Matthijs was asking about working properly.  Apparently
a call to Tk's "update" (basically just let Tk pump its event loop)
is needed after I've finished reading the file content, but before I
adjust the view.  Its in my pu branch now (gitgui-0.7.2-58-gf9e96fd).

-- 
Shawn.

Re: Improved git-gui blame viewer

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

hoi :)

On Mon, Jun 04, 2007 at 04:21:56AM -0400, Shawn O. Pearce wrote:
> I think so, but its ugly.  The viewer is actually 4 text widgets
> crammed next to each other.  I can set the background color of a
> line by giving it a tag, so to do a gradient I have to assign a
> different background color to each line by giving each line its
> own tag (ick).  Worse, in a 3 line chunk I can only do 3 colors.
> That fails your "smooth" concept.  ;-)

yeah, if each line has a solid background that does not work :-(

> > Perhaps a simple small line between hunks is enough, too?
> 
> That would be messy.  I can certainly cause a few pixels of spacing
> to show up between chunks, but I'm reading the data "live" from the
> blame engine and putting it on screen.  Adding space betwen chunks
> as I get it will cause the data to "reflow" while you are trying to
> read it.  I can probably account for it with the scrollbar and adjust
> it accordingly, but at some point you will wind up seeing the text
> in the viewer pane moving around and expanding as the padding gets
> tossed in.

Well, it would work if you could just draw a one-pixel line (in some
subtle gray) inbetween lines, without changing the layout.

> BTW, I just got the jump-to-original line and restore-view-on-back
> features that Matthijs was asking about working properly.  Apparently
> a call to Tk's "update" (basically just let Tk pump its event loop)
> is needed after I've finished reading the file content, but before I
> adjust the view.  Its in my pu branch now (gitgui-0.7.2-58-gf9e96fd).

nice :-)

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Improved git-gui blame viewer

[Matthijs Melchior]

Shawn O. Pearce wrote:
> Matthijs Melchior <mmelchior@xs4all.nl> wrote:
....
> 
>>   Something I want for the normal window, in the Staged and Unstaged
>>   file lists, high-lite the last entry selected so it becomes easy to
>>   click on the next one and I can see more clearly what is displayed
>>   in the bottom area.
> 
> I'm not sure I understand what you are looking for here.  Right now
> git-gui should be inverting the foreground/background colors on
> the file that is "selected" (shown in the lower diff view pane).
> So the background should be black, and the foreground white.
> Is this not happening?  Or are you looking for something else?
> 

No, I am not looking for something else...., the inverting you describe
does not happen on my machine....

I am now running Debian git-core 1.5.2.1-1 with 'make install' done
in the origin/pu branch of git-gui.
'About git-gui' now says:
	git-gui version 0.7.2.58-gf9e9
	git version 1.5.2.1
	Tcl/Tk version 8.4.12

If you explain where this inverting is taking place, I can do some
experiments to find out more [use gray background i.s.o. inverting...]
Maybe it has something to do with Desktop themes, I use the standard
Gnome theme.

Thanks,
	Matthijs Melchior.

Re: Improved git-gui blame viewer

[Shawn O. Pearce]

Matthijs Melchior <mmelchior@xs4all.nl> wrote:
> Shawn O. Pearce wrote:
> > I'm not sure I understand what you are looking for here.  Right now
> > git-gui should be inverting the foreground/background colors on
> > the file that is "selected" (shown in the lower diff view pane).
> > So the background should be black, and the foreground white.
> > Is this not happening?  Or are you looking for something else?
> > 
> 
> No, I am not looking for something else...., the inverting you describe
> does not happen on my machine....

I'm wrong.  Its not inverting.  Its bold if its selected, and normal
if its not selected.  Perhaps your font is already a bold weight
so you aren't seeing a difference between the selected item and
the non-selected items.
 
> I am now running Debian git-core 1.5.2.1-1 with 'make install' done
> in the origin/pu branch of git-gui.
> 'About git-gui' now says:
> 	git-gui version 0.7.2.58-gf9e9
> 	git version 1.5.2.1
> 	Tcl/Tk version 8.4.12
> 
> If you explain where this inverting is taking place, I can do some
> experiments to find out more [use gray background i.s.o. inverting...]
> Maybe it has something to do with Desktop themes, I use the standard
> Gnome theme.

Around line 1803 of git-gui.sh we setup the in_diff tag for the
$ui_index and $ui_workdir Tk widgets.  That tag is applied to the
file that is in the diff viewer.  Perhaps adding a background to
the tag would get you an improved interface?

-- 
Shawn.

Re: Improved git-gui blame viewer

[Matthijs Melchior]

Shawn O. Pearce wrote:
> Matthijs Melchior <mmelchior@xs4all.nl> wrote:
>   
>> Shawn O. Pearce wrote:
>>     
>>> I'm not sure I understand what you are looking for here.  Right now
>>> git-gui should be inverting the foreground/background colors on
>>> the file that is "selected" (shown in the lower diff view pane).
>>> So the background should be black, and the foreground white.
>>> Is this not happening?  Or are you looking for something else?
>>>
>>>       
>> No, I am not looking for something else...., the inverting you describe
>> does not happen on my machine....
>>     
>
> I'm wrong.  Its not inverting.  Its bold if its selected, and normal
> if its not selected.  Perhaps your font is already a bold weight
> so you aren't seeing a difference between the selected item and
> the non-selected items.
>  
>   
>> I am now running Debian git-core 1.5.2.1-1 with 'make install' done
>> in the origin/pu branch of git-gui.
>> 'About git-gui' now says:
>> 	git-gui version 0.7.2.58-gf9e9
>> 	git version 1.5.2.1
>> 	Tcl/Tk version 8.4.12
>>
>> If you explain where this inverting is taking place, I can do some
>> experiments to find out more [use gray background i.s.o. inverting...]
>> Maybe it has something to do with Desktop themes, I use the standard
>> Gnome theme.
>>     
>
> Around line 1803 of git-gui.sh we setup the in_diff tag for the
> $ui_index and $ui_workdir Tk widgets.  That tag is applied to the
> file that is in the diff viewer.  Perhaps adding a background to
> the tag would get you an improved interface?
>   
Yes, this is the problem.

I will send you a patch to change the background of the selected file
to lightgray.

I have included in that patch some softer colors for the headers of
the three involved widgets. I hope you like these.

Thanks.

-- 
Regards,
----------------------------------------------------------------  -o)
Matthijs Melchior                                       Maarssen  /\\
mmelchior@xs4all.nl                                  Netherlands _\_v
---------------------------------------------------------------- ----

Re: Improved git-gui blame viewer

[Alex Riesen]

On 6/2/07, Shawn O. Pearce <spearce@spearce.org> wrote:
>
> I finally got the git-gui code to the point where cleaning up the
> user interface was possible without sending myself to the nut house.
>

Very-very nice :) Does not seem to save sizes and positions of
blame and file browser windows, though. It did before, I believe
in .git/config.

BTW, saving windows positions in .git/config was scary: I
considered it user domain (yes, I _am_ afraid of using
git-config too).
Maybe it could be something like either ~/.git-gui or .git/guiconfig?

Re: Improved git-gui blame viewer

[Shawn O. Pearce]

Alex Riesen <raa.lkml@gmail.com> wrote:
> On 6/2/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> >
> >I finally got the git-gui code to the point where cleaning up the
> >user interface was possible without sending myself to the nut house.
> >
> 
> Very-very nice :) 

Thanks.  I'm not done yet.  There's a lot of interesting navigation
I still want to get in, like visting the parent of a changed line,
instead of the commit that changed that line.  I also want to let
you open commits in a new window, rather than the current one.

> Does not seem to save sizes and positions of
> blame and file browser windows, though. It did before, I believe
> in .git/config.

Actually we have never saved the window sizes/positions of the 
blame and browser windows, only the main commit window.  Though
you may have been seeing a bug where we restored the main commit
window's saved geometry on a browser or blame window if the latter
was started from the command line...

> BTW, saving windows positions in .git/config was scary: I
> considered it user domain (yes, I _am_ afraid of using
> git-config too).
> Maybe it could be something like either ~/.git-gui or .git/guiconfig?

I'm apparently not afraid of git-config editing file(s).  I have
yet to see a failure from it.  I guess I'm lucky, but will now
suffer a failure tomorrow when I'm least expecting it.  ;-)

Don't we use git-config to edit the config file in git-branch?
In git-remote?  git-gui has *always* used git-config to store
its data.  You are sort of asking me to replace a working config
system with a new one that hasn't been tested...

Now moving the git-gui config to say ~/.gitgui-config and
.git/gitgui-config may have a good argument, as then git-gui
is editing its own files.  Unless the user changes user.name
or user.email through git-gui, in which case we have to edit
.git/config or ~/.gitconfig anyway...

-- 
Shawn.

Re: Improved git-gui blame viewer

[Alex Riesen]

On 6/5/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Don't we use git-config to edit the config file in git-branch?

Not routinely. Seldom, even

> In git-remote?  git-gui has *always* used git-config to store
> its data.  You are sort of asking me to replace a working config
> system with a new one that hasn't been tested...

Just divert the data someplace else.

> Now moving the git-gui config to say ~/.gitgui-config and
> .git/gitgui-config may have a good argument, as then git-gui
> is editing its own files.

Good argument on its own

>  Unless the user changes user.name
> or user.email through git-gui, in which case we have to edit
> .git/config or ~/.gitconfig anyway...

Yes, then of course (in this case the user is even aware).