git gui diff widget (was Re: [msysGit] Re: [cheetah] Questions about NOTES)

git gui diff widget (was Re: [msysGit] Re: [cheetah] Questions about NOTES)

[Steffen Prohaska]

[ I put the git list on CC and chose a new topic. ]


On Jan 30, 2008, at 5:03 AM, Shawn O. Pearce wrote:

>
> Johannes Schindelin <Johannes.Schindelin-Mmb7MZpHnFY@public.gmane.org> wrote:
>> On Mon, 28 Jan 2008, Shawn O. Pearce wrote:
>>> Johannes Schindelin <Johannes.Schindelin-Mmb7MZpHnFY@public.gmane.org> wrote:
>>>>
>>>> Another thing (and this concerns you, Shawn): there is this lovely
>>>> program called git-gui, and it has most of what we will ever  
>>>> want to
>>>> drive from git-cheetah.  Is there any method to call git-gui  
>>>> such that
>>>> it performs a certain command, or is there a chance to get such a
>>>> method?
>>
>> I did not mean to remote control it, just to start it with a default
>> action, such as "diff file <xyz> to HEAD", or "diff directory  
>> <xyz> to
>> index".
>
> Oh, like we do for `git gui blame HEAD file` ?  :-)
>
> That isn't too difficult.  git-gui currently cannot show a diff of
> a directory, it can only diff a single file at a time, but yea,
> it wouldn't be hard to add a `git gui diff file` to have it open
> up and show the current difference of index<->file or HEAD<->index
> for that path.
>
> Doing a diff against a random other tree-ish, or between two random
> tree-ishes would be possible, but a lot more of a challenge.
>
> The diff viewer in git-gui isn't exactly a reusable widget.
> We'd need to refactor that so it could take patch output from any
> of the patch generating git commands.  Doing such is on my list
> of things I'd like to fix in git-gui, but I haven't had time to
> do it yet.

Hmm, I'm not sure if such a full diff viewer is the most
important thing to address.  Most users (especially Windows
users) already have a favorite graphical diff tool and they expect
a mechanism to plug it into a git workflow.

A difficulty they face is that in a git workflow they often need
to diff between two trees and this case is currently not handled
in a sensible way if you want to view the diffs graphically.
Well you could set GIT_EXTERNAL_DIFF to a graphical tool.  But
then git would pop-up a new instance of your graphical tool for
every single file and this is certainly not what you expect (I
tried it once and immediately stopped this experiment).

I think a tool that presents the list of files with differences
and can launch an external tool for one of these files would be
very helpful.  I started to think about a command line version,
git-difftool, implementing this idea, but do not yet have code.
A GUI version would be preferable for cheetah anyway.  Maybe the
patch viewer you sketched above could provide a way to launch an
external tool for any of the files touched by the patch?  If the
widget would be reusable maybe it could also be used in gitk?

	Steffen

Re: git gui diff widget (was Re: [msysGit] Re: [cheetah] Questions about NOTES)

[Shawn O. Pearce]

Steffen Prohaska <prohaska-wjoc1KHpMeg@public.gmane.org> wrote:
> On Jan 30, 2008, at 5:03 AM, Shawn O. Pearce wrote:
> >
> >Doing a diff against a random other tree-ish, or between two random
> >tree-ishes would be possible, but a lot more of a challenge.
> >
> >The diff viewer in git-gui isn't exactly a reusable widget.
> >We'd need to refactor that so it could take patch output from any
> >of the patch generating git commands.  Doing such is on my list
> >of things I'd like to fix in git-gui, but I haven't had time to
> >do it yet.
> 
> Hmm, I'm not sure if such a full diff viewer is the most
> important thing to address.  Most users (especially Windows
> users) already have a favorite graphical diff tool and they expect
> a mechanism to plug it into a git workflow.

I hear this a lot from a co-worker.  He really wants a side-by-side
diff tool in git, as he doesn't like reading patches.  Me, I've
never been able to read a side-by-side diff tool, but I can grok
a unified diff quite easily.  To each his own, and git should help
users as much as it can.
 
> I think a tool that presents the list of files with differences
> and can launch an external tool for one of these files would be
> very helpful.  I started to think about a command line version,
> git-difftool, implementing this idea, but do not yet have code.
> A GUI version would be preferable for cheetah anyway.  Maybe the
> patch viewer you sketched above could provide a way to launch an
> external tool for any of the files touched by the patch?  If the
> widget would be reusable maybe it could also be used in gitk?

Yea, that makes sense.  But we may want to go the other way,
that is reuse gitk's patch viewer.  Or something.  The two tools
(git-gui and gitk) grew their diff viewers independently, though
I have to say that git-gui's was inspired by gitk's, and the work
that Paul did on gitool waaaaay back when.

Right now though I doubt there's a single line of code in common
between them.  Hmm, I think they both use the Tk "text" widget.
And a blue-ish color for hunk headers.  :)


Adding a feature to git-gui to launch an external diff of that
file against the index or HEAD shouldn't be too difficult, and I
think is what would be most natural for the current UI.  The ugly
part is pulling out the HEAD/index version into a temporary file
and passing a nice -L (or something like) option to the user's diff
viewer so they don't see the nasty temporary file name, but instead
see a string that matches what git-gui is showing them.  And that
option is going to differ for like every tool out there.  :-\

-- 
Shawn.

Re: git gui diff widget (was Re: [msysGit] Re: [cheetah] Questions about NOTES)

[Thomas Arcila]

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

Hi,

Here is a patch to gitk that allows to run an external diff viewer.
I think it might be a suitable solution. 
It can be configured in Edit/Preferences/External diff tool.

To see the diff between two files:
- select revisions to diff
- right click on a file in the patched files list view
- choose "External diff"

Any feedback is welcome.
Thomas

Le mercredi 30 janvier 2008 à 02:13 -0500, Shawn O. Pearce a écrit :
> Steffen Prohaska <prohaska-wjoc1KHpMeg@public.gmane.org> wrote:
> > On Jan 30, 2008, at 5:03 AM, Shawn O. Pearce wrote:
> > >
> > >Doing a diff against a random other tree-ish, or between two random
> > >tree-ishes would be possible, but a lot more of a challenge.
> > >
> > >The diff viewer in git-gui isn't exactly a reusable widget.
> > >We'd need to refactor that so it could take patch output from any
> > >of the patch generating git commands.  Doing such is on my list
> > >of things I'd like to fix in git-gui, but I haven't had time to
> > >do it yet.
> > 
> > Hmm, I'm not sure if such a full diff viewer is the most
> > important thing to address.  Most users (especially Windows
> > users) already have a favorite graphical diff tool and they expect
> > a mechanism to plug it into a git workflow.
> 
> I hear this a lot from a co-worker.  He really wants a side-by-side
> diff tool in git, as he doesn't like reading patches.  Me, I've
> never been able to read a side-by-side diff tool, but I can grok
> a unified diff quite easily.  To each his own, and git should help
> users as much as it can.
>  
> > I think a tool that presents the list of files with differences
> > and can launch an external tool for one of these files would be
> > very helpful.  I started to think about a command line version,
> > git-difftool, implementing this idea, but do not yet have code.
> > A GUI version would be preferable for cheetah anyway.  Maybe the
> > patch viewer you sketched above could provide a way to launch an
> > external tool for any of the files touched by the patch?  If the
> > widget would be reusable maybe it could also be used in gitk?
> 
> Yea, that makes sense.  But we may want to go the other way,
> that is reuse gitk's patch viewer.  Or something.  The two tools
> (git-gui and gitk) grew their diff viewers independently, though
> I have to say that git-gui's was inspired by gitk's, and the work
> that Paul did on gitool waaaaay back when.
> 
> Right now though I doubt there's a single line of code in common
> between them.  Hmm, I think they both use the Tk "text" widget.
> And a blue-ish color for hunk headers.  :)
> 
> 
> Adding a feature to git-gui to launch an external diff of that
> file against the index or HEAD shouldn't be too difficult, and I
> think is what would be most natural for the current UI.  The ugly
> part is pulling out the HEAD/index version into a temporary file
> and passing a nice -L (or something like) option to the user's diff
> viewer so they don't see the nasty temporary file name, but instead
> see a string that matches what git-gui is showing them.  And that
> option is going to differ for like every tool out there.  :-\
> 

[-- Attachment #2: 0001-gitk-External-diff-viewer.patch --]
[-- Type: application/mbox, Size: 6880 bytes --]

Re: git gui diff widget (was Re: [msysGit] Re: [cheetah] Questions about NOTES)

[Steffen Prohaska]

On Jan 30, 2008, at 8:58 AM, Thomas Arcila wrote:

> Here is a patch to gitk that allows to run an external diff viewer.
> I think it might be a suitable solution.
> It can be configured in Edit/Preferences/External diff tool.
>
> To see the diff between two files:
> - select revisions to diff
> - right click on a file in the patched files list view
> - choose "External diff"
>
> Any feedback is welcome.

The basic functionality works for me, however ...


> Subject: [PATCH] gitk : External diff viewer.
>
> Right click on patched file list view gives "External diff" popup  
> menu entry,
> launching selected external diff tool.
>
> Signed-off-by: Thomas Arcila <tarcila-s5SjKpinsOs@public.gmane.org>
> ---
>  gitk |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 
> ++++++++--
>  1 files changed, 105 insertions(+), 4 deletions(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 5560e4d..0e2c902 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk

[...]

> @@ -1747,6 +1762,78 @@ proc flist_hl {only} {
>      set gdttype [mc "touching paths:"]
>  }
>
> +proc external_diff {} {
> +  global gitktmpdir nullid nullid2
> +  global flist_menu_file
> +  global diffids
> +  global diffnum
> +  global gitdir extdifftool
> +
> +  set diffidto [lindex $diffids 0]
> +
> +  if {[llength $diffids] == 1} {
> +    # no reference commit given
> +    set diffidto [lindex $diffids 0]
> +    if {$diffidto eq $nullid || $diffidto eq $nullid2} {
> +      # diffing working copy with HEAD

That's not quite true.  nullid refers to the work tree but
nullid2 refers to the index. So
  - if $diffidto eq $nullid the work tree needs to be
    compared against the index
  - if $diffidto eq $nullid2 the index needs to be compared
    against HEAD.


> +      set diffidfrom "HEAD"
> +    } else {
> +      # use parent commit
> +      global allparents
> +      set diffidfrom $allparents($diffidto)
> +    }
> +  } else {
> +    set diffidfrom [lindex $diffids 0]
> +    set diffidto [lindex $diffids 1]
> +  }
> +
> +  if {! [info exists diffnum]} {
> +    set diffnum 0
> +  } else {
> +    set diffnum [expr $diffnum + 1]
> +  }
> +
> +
> +  set diffdir [file join $gitktmpdir "$diffnum"]
> +  set diffok "true"
> +
> +  file mkdir $diffdir
> +  if {$diffidto eq $nullid || $diffidto eq $nullid2} {
> +    set difftofile [file join $gitdir ".." $flist_menu_file]

This needs to be modified, too.


> +  } {
> +    set difftofile [file join $diffdir "\[$diffidto\] [file tail  
> $flist_menu_file]"]
> +    if {[catch {exec git show $diffidto:$flist_menu_file >  
> $difftofile} err]} {
> +      error_popup "\"$flist_menu_file\" [mc "cannot be found in  
> revision"] $diffidto. [mc "File has probably been created, \
> +                  deleted or renamed, in a different commit."]"
> +      set diffok "false"
> +    }
> +  }
> +
> +  if {$diffidfrom == $nullid || $diffidfrom == $nullid2} {
> +    set difffromfile [file join $gitdir ".." $flist_menu_file]

ditto.


> +  } else {
> +    set difffromfile [file join $diffdir "\[$diffidfrom\] [file  
> tail $flist_menu_file]"]
> +    if {[catch {exec git show $diffidfrom:$flist_menu_file >  
> $difffromfile} err]} {
> +      error_popup "\"$flist_menu_file\" [mc "cannot be found in  
> revision"] $diffidfrom. [mc "File has probably been created, \
> +                  deleted or renamed, in a different commit."]"
> +      set diffok "false"
> +    }
> +  }
> +
> +  if {$diffok} {
> +    # here we don't use shellquote because \ and everything must  
> be escaped and not enclosed between ''
> +    set quotedextdifftool \"[string map {\" \\\" \\ \\\\ \  \\\ }  
> $extdifftool]\"
> +    set cmd [concat | $quotedextdifftool [shellarglist [list  
> $difffromfile $difftofile]]]
> +    if {[catch {set fl [open $cmd]} err]} {
> +      file delete -force [ file join $gitktmpdir $diffnum ]
> +      error_popup [mc "$extdifftool command failed: $err"]
> +    } else {
> +      fconfigure $fl -blocking 0
> +      filerun $fl [list file delete -force [file join $gitktmpdir  
> $diffnum]]
> +    }
> +  }
> +}
> +
>

>  # Functions for adding and removing shell-type quoting
>
>  proc shellquote {str} {
> @@ -7802,9 +7889,13 @@ proc showtag {tag isnew} {
>
>  proc doquit {} {
>      global stopped
> +    global gitktmpdir
> +
>      set stopped 100
>      savestuff .
>      destroy .
> +
> +    catch {file delete -force $gitktmpdir}
>  }
>
>  proc mkfontdisp {font top which} {
> @@ -7933,7 +8024,7 @@ proc doprefs {} {
>      global maxwidth maxgraphpct
>      global oldprefs prefstop showneartags showlocalchanges
>      global bgcolor fgcolor ctext diffcolors selectbgcolor
> -    global tabstop limitdiffs
> +    global tabstop limitdiffs extdifftool
>
>      set top .gitkprefs
>      set prefstop $top
> @@ -7980,6 +8071,11 @@ proc doprefs {} {
>      pack $top.ldiff.b $top.ldiff.l -side left
>      grid x $top.ldiff -sticky w
>
> +    entry $top.extdifft -textvariable extdifftool
> +    button $top.extdiffb -text [mc "External diff tool" ] -font  
> optionfont -command {set extdifftool [tk_getOpenFile -title  
> "External diff tool" -multiple "false"]}

Could you wrap the line using \

	Steffen