{"thread":{"id":"21632","subject":"[PATCH] gitk: Use git-difftool for external diffs","startedAt":"2009-11-16T03:12:06Z","lastAt":"2009-12-31T20:37:31Z","messageCount":13,"participants":["David Aguilar","Paul Mackerras","Junio C Hamano","Markus Heidelberg","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127656","messageId":"1258341126-2108-1-git-send-email-davvid@gmail.com","threadId":"21632","inReplyTo":null,"subject":"[PATCH] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-11-16T03:12:06Z","receivedAt":"2009-11-16T03:12:06Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This teaches gitk about git-difftool. A benefit of this change is\nthat gitk's external diff now works with read-only repositories.\n\nSigned-off-by: David Aguilar \n---\n\nThe diffstat alone makes me happy.\n\n gitk | 83 ++++--------------------------------------------------------------\n 1 files changed, 5 insertions(+), 78 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex db5ec54..881ef95 100755\n--- a/gitk\n+++ b/gitk\n@@ -2520,7 +2520,7 @@ proc savestuff {w} {\n global viewname viewfiles viewargs viewargscmd viewperm nextviewnum\n global cmitmode wrapcomment datetimeformat limitdiffs\n global colors uicolor bgcolor fgcolor diffcolors diffcontext selectbgcolor\n- global autoselect extdifftool perfile_attrs markbgcolor\n+ global autoselect perfile_attrs markbgcolor\n global hideremotes\n \n if {$stuffsaved} return\n@@ -2554,7 +2554,6 @@ proc savestuff {w} {\n \tputs $f [list set markbgcolor $markbgcolor]\n \tputs $f [list set diffcontext $diffcontext]\n \tputs $f [list set selectbgcolor $selectbgcolor]\n-\tputs $f [list set extdifftool $extdifftool]\n \tputs $f [list set perfile_attrs $perfile_attrs]\n \n \tputs $f \"set geometry(main) [wm geometry .]\"\n@@ -3204,31 +3203,10 @@ proc save_file_from_commit {filename output what} {\n return $output\n }\n \n-proc external_diff_get_one_file {diffid filename diffdir} {\n- global nullid nullid2 nullfile\n- global gitdir\n-\n- if {$diffid == $nullid} {\n- set difffile [file join [file dirname $gitdir] $filename]\n-\tif {[file exists $difffile]} {\n-\t return $difffile\n-\t}\n-\treturn $nullfile\n- }\n- if {$diffid == $nullid2} {\n- set difffile [file join $diffdir \"\\[index\\] [file tail $filename]\"]\n- return [save_file_from_commit :$filename $difffile index]\n- }\n- set difffile [file join $diffdir \"\\[$diffid\\] [file tail $filename]\"]\n- return [save_file_from_commit $diffid:$filename $difffile \\\n-\t \"revision $diffid\"]\n-}\n-\n proc external_diff {} {\n global nullid nullid2\n global flist_menu_file\n global diffids\n- global extdifftool\n \n if {[llength $diffids] == 1} {\n # no reference commit given\n@@ -3249,23 +3227,9 @@ proc external_diff {} {\n set diffidto [lindex $diffids 1]\n }\n \n- # make sure that several diffs wont collide\n- set diffdir [gitknewtmpdir]\n- if {$diffdir eq {}} return\n-\n- # gather files to diff\n- set difffromfile [external_diff_get_one_file $diffidfrom $flist_menu_file $diffdir]\n- set difftofile [external_diff_get_one_file $diffidto $flist_menu_file $diffdir]\n-\n- if {$difffromfile ne {} && $difftofile ne {}} {\n- set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]\n- if {[catch {set fl [open |$cmd r]} err]} {\n- file delete -force $diffdir\n- error_popup \"$extdifftool: [mc \"command failed:\"] $err\"\n- } else {\n- fconfigure $fl -blocking 0\n- filerun $fl [list delete_at_eof $fl $diffdir]\n- }\n+ set cmd {git difftool --no-prompt $diffidfrom $diffidto -- $flist_menu_file}\n+ if {[catch {eval exec $cmd &} err]} {\n+ error_popup \"[mc \"git difftool: command failed:\"] $err\"\n }\n }\n \n@@ -3557,19 +3521,6 @@ proc read_line_source {fd inst} {\n return 0\n }\n \n-# delete $dir when we see eof on $f (presumably because the child has exited)\n-proc delete_at_eof {f dir} {\n- while {[gets $f line] >= 0} {}\n- if {[eof $f]} {\n-\tif {[catch {close $f} err]} {\n-\t error_popup \"[mc \"External diff viewer failed:\"] $err\"\n-\t}\n-\tfile delete -force $dir\n-\treturn 0\n- }\n- return 1\n-}\n-\n # Functions for adding and removing shell-type quoting\n \n proc shellquote {str} {\n@@ -10446,7 +10397,7 @@ proc doprefs {} {\n global maxwidth maxgraphpct\n global oldprefs prefstop showneartags showlocalchanges\n global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor\n- global tabstop limitdiffs autoselect extdifftool perfile_attrs\n+ global tabstop limitdiffs autoselect perfile_attrs\n global hideremotes\n \n set top .gitkprefs\n@@ -10498,15 +10449,6 @@ proc doprefs {} {\n \t-font optionfont -variable perfile_attrs\n grid x $top.lattr -sticky w\n \n- entry $top.extdifft -textvariable extdifftool\n- frame $top.extdifff\n- label $top.extdifff.l -text [mc \"External diff tool\" ] -font optionfont \\\n-\t-padx 10\n- button $top.extdifff.b -text [mc \"Choose...\"] -font optionfont \\\n-\t-command choose_extdiff\n- pack $top.extdifff.l $top.extdifff.b -side left\n- grid x $top.extdifff $top.extdifft -sticky w\n-\n label $top.cdisp -text [mc \"Colors: press to choose\"]\n grid $top.cdisp - -sticky w -pady 10\n label $top.ui -padx 40 -relief sunk -background $uicolor\n@@ -10566,15 +10508,6 @@ proc doprefs {} {\n bind $top \"focus $top.buts.ok\"\n }\n \n-proc choose_extdiff {} {\n- global extdifftool\n-\n- set prog [tk_getOpenFile -title [mc \"External diff tool\"] -multiple false]\n- if {$prog ne {}} {\n-\tset extdifftool $prog\n- }\n-}\n-\n proc choosecolor {v vi w x cmd} {\n global $v\n \n@@ -11100,12 +11033,6 @@ set datetimeformat \"%Y-%m-%d %H:%M:%S\"\n set autoselect 1\n set perfile_attrs 0\n \n-if {[tk windowingsystem] eq \"aqua\"} {\n- set extdifftool \"opendiff\"\n-} else {\n- set extdifftool \"meld\"\n-}\n-\n set colors {green red blue magenta darkgrey brown orange}\n set uicolor grey85\n set bgcolor white\n-- \n1.6.5.2.180.gc5b3e\n"},{"id":"127891","messageId":"19205.2531.205062.980468@cargo.ozlabs.ibm.com","threadId":"21632","inReplyTo":"1258341126-2108-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2009-11-19T09:03:31Z","receivedAt":"2009-11-19T09:03:31Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"David Aguilar writes:\n\n> This teaches gitk about git-difftool. A benefit of this change is\n> that gitk's external diff now works with read-only repositories.\n\nWhat version of git does git difftool first appear in? I prefer not\nto introduce hard requirements on very recent versions of git into\ngitk.\n\nPaul.\n"},{"id":"127920","messageId":"20091119193913.GA25410@gmail.com","threadId":"21632","inReplyTo":"19205.2531.205062.980468@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-11-19T19:39:15Z","receivedAt":"2009-11-19T19:39:15Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Nov 19, 2009 at 08:03:31PM +1100, Paul Mackerras wrote:\n> David Aguilar writes:\n> \n> > This teaches gitk about git-difftool. A benefit of this change is\n> > that gitk's external diff now works with read-only repositories.\n> \n> What version of git does git difftool first appear in? I prefer not\n> to introduce hard requirements on very recent versions of git into\n> gitk.\n> \n> Paul.\n\ngit-difftool appeared in git 1.6.3.\n\nIf this patch is not going in then how do you suggest we fix the\nread-only repository bug?\n\nMy immediate thought is to harden the $TMPDIR patch so that the\nfilenames used by gitk are much less predictable (I bailed on it\nonce Peff noted the $TMPDIR vulnerabilities and used\ngit-difftool instead since it is known to be safe).\n\nDoes hardening the $TMPDIR patch have a better chance of\nbeing accepted?\n\n\nIn defense of difftool: there are more benefits to using\ngit-difftool than just read-only repositories.\nThe current external diff code does not work if 'meld'\nis not installed whereas using git-difftool works by default\nin more environments. It also makes things match users'\nexpectations when they have already gone through the trouble\nof configuring a diff or merge tool.\n\nIs there a particular time frame in which such a patch could go\nin?\n\n\nMy primary concern is getting gitk to work with read-only\nrepositories. git-difftool was just one way of getting there.\nIf keeping backwards compatibility is a must-have then I can\nhold onto the git-difftool patch until some time in the future\nwhen it is more appropriate. Hopefully something along the\nlines of a hardened $TMPDIR patch can stand in for the final\nfix in the meantime.\n\nWhat do you think?\n\n-- \n\t\tDavid\n"},{"id":"127936","messageId":"19205.50406.91209.309984@cargo.ozlabs.ibm.com","threadId":"21632","inReplyTo":"20091119193913.GA25410@gmail.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2009-11-19T22:21:26Z","receivedAt":"2009-11-19T22:21:26Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"David Aguilar writes:\n\n> On Thu, Nov 19, 2009 at 08:03:31PM +1100, Paul Mackerras wrote:\n> > David Aguilar writes:\n> > \n> > > This teaches gitk about git-difftool. A benefit of this change is\n> > > that gitk's external diff now works with read-only repositories.\n> > \n> > What version of git does git difftool first appear in? I prefer not\n> > to introduce hard requirements on very recent versions of git into\n> > gitk.\n> > \n> > Paul.\n> \n> git-difftool appeared in git 1.6.3.\n> \n> If this patch is not going in then how do you suggest we fix the\n> read-only repository bug?\n\nI have no problem with using git difftool if the underlying git is new\nenough, I just don't want gitk to explode when it isn't.\n\nIf the underlying git isn't new enough then we should probably make a\ndirectory under $TMPDIR with a reasonably unpredictable name.\n\nAlso, I don't think we should remove the ability for the user to\nchoose which external diff tool to use; if we're using git difftool\nthen we should pass the selected tool with the -t option of git\ndifftool. Maybe we need a \"use default from git config\" option\nas well, though.\n\nPaul.\n"},{"id":"127982","messageId":"7vhbsp7htq.fsf@alter.siamese.dyndns.org","threadId":"21632","inReplyTo":"19205.50406.91209.309984@cargo.ozlabs.ibm.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-20T07:53:37Z","receivedAt":"2009-11-20T07:53:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras writes:\n\n> I have no problem with using git difftool if the underlying git is new\n> enough, I just don't want gitk to explode when it isn't.\n\nI somehow doubt there are many users who use pre 1.6.3 git but keep their\ngitk separately updated to very recent version, so personally I wouldn't\nworry too much about this.\n\n> Also, I don't think we should remove the ability for the user to\n> choose which external diff tool to use.\n\nThis is a larger concern. Does \"difftool\" allow use of an arbitrary tool\nlike how gitk does (I have a suspicion that it is not as flexible)?\n"},{"id":"128035","messageId":"20091120185522.GC56351@gmail.com","threadId":"21632","inReplyTo":"7vhbsp7htq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-11-20T18:55:24Z","receivedAt":"2009-11-20T18:55:24Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Nov 19, 2009 at 11:53:37PM -0800, Junio C Hamano wrote:\n> Paul Mackerras writes:\n> \n> > I have no problem with using git difftool if the underlying git is new\n> > enough, I just don't want gitk to explode when it isn't.\n> \n> I somehow doubt there are many users who use pre 1.6.3 git but keep their\n> gitk separately updated to very recent version, so personally I wouldn't\n> worry too much about this.\n> \n> > Also, I don't think we should remove the ability for the user to\n> > choose which external diff tool to use.\n> \n> This is a larger concern. Does \"difftool\" allow use of an arbitrary tool\n> like how gitk does (I have a suspicion that it is not as flexible)?\n\ndifftool supports arbitrary tools through configuration.\nFor arbitrary tools we set difftool.$name.cmd and\ndiff.tool = $name.\n\nIf gitk were to have a difftool paradigm then it might\nbe nice to have a text field mapped to the diff.tool variable.\nThe patch didn't address the diff.tool configuration variable\nso that's a concern if we are to pursue difftool further.\n\nThe argument in favor of difftool is one of user\nexpectations. From a user's POV it ~seems~ desirable for\ngitk to honor difftool configurations. The easiest way for\ngitk to do that is to use git-difftool and not worry about\nthe details of how arbitrary tools are setup.\n\nLater work could be done to provide UI for setting up\narbitrary tools inside of gitk. My gut feeling\nis that gitk isn't a git config editor.\n\nThis is why the difftool patch took the minimalist approach.\nIt trusts the user to have things configured to their\nliking and trusts git-difftool to do the right thing\nwhen no such configuration exists.\n\nI think it's a question of whether this is something we'd\nwant to change. It certainly doesn't bother me either way.\n\nI might be the only person who'd notice such small details\nso my vote-in-code is with my v2 $TMPDIR patch until someone\nelse thinks that using difftool inside of gitk is a good idea.\n\nAt which point I would probably pursue it with the minimalist\napproach first and then gradually add UI for the pertinent\ngit config variables.\n\nJust my .02,\n\n-- \n\t\tDavid\n"},{"id":"128047","messageId":"7veinsx6lj.fsf@alter.siamese.dyndns.org","threadId":"21632","inReplyTo":"20091120185522.GC56351@gmail.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-20T20:51:36Z","receivedAt":"2009-11-20T20:51:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n>> > Also, I don't think we should remove the ability for the user to\n>> > choose which external diff tool to use.\n>> \n>> This is a larger concern. Does \"difftool\" allow use of an arbitrary tool\n>> like how gitk does (I have a suspicion that it is not as flexible)?\n>\n> difftool supports arbitrary tools through configuration.\n> For arbitrary tools we set difftool.$name.cmd and\n> diff.tool = $name.\n\nSo it is not as flexible as just giving a command line template for\none-shot invocation, but that is Ok.\n\nDon't get me wrong. I think the longer term direction should be to reduce\ncode from gitk without making lifes harder to existing users, and just\nlike built-in \"diff --cc\" stripped the combined-diff implemented in Tcl\nfrom gitk, use of \"difftool\" would be a good way to unify the \"diffing\"\nexperience for a user who uses both command line and gitk at the same\ntime.\n\nI do not read Tcl very well but I am guessing that in gitk you specify\nwhat tool to run (e.g. \"frobanodiff -z\"), gitk feeds you two temporary\nfiles on the filesystem to compare (e.g. \"frobanodiff -z $tmp1 $tmp2\"),\nand your command line is responsible for giving satisfying diff experience\nto the end user.\n\nI see three possible approaches:\n\n * Teach \"git-difftool\" a mode to compare two arbitrary files on the\n filesytem, and set that as \"External Diff\" command that takes the\n filenames as extra two parameters, just like any other \"External Diff\"\n programs given to gitk does. This is the least palatable, as it won't\n solve the read-only repository issue at all (it only allows you the\n logic to choose the configured difftool backend program).\n\n * Instead of disabling the traditional \"External Diff\" and taking it over\n like your patch did, add a new codepath for \"Difftool\" that feeds the\n commit IDs and paths the way git-difftool expects. The user can use\n both, and the issue of read-only repository is solved when \"Difftool\"\n is used (but not \"External Diff\").\n\n * Take over \"External Diff\" codepath exactly like your patch did, but\n teach \"git-difftool\" a new command line option to name an unconfigured\n external program that takes two filenames. When \"External Diff\"\n program is *not* configured in gitk, the command line to invoke\n difftool would be exactly as in your patch, i.e. \"difftool --no-prompt\n $from $to -- $path\". Otherwise, when gitk is configured to use an\n external program, e.g. \"frobanodiff -z\", for \"External Diff\", you pass\n that command line to \"git-difftool\" via that new option, e.g.\n\n difftool --no-prompt --extcmd=\"frobanodiff -z\" $from $to -- $path\n\n Then difftool is responsible for preparing the two necessary temporary\n files out of the given information ($from/$to/$path) and feeding them\n to \"frobanodiff -z\" command line.\n\n Maybe such --extcmd support already exists in difftool, in which case\n my earlier suspicion that difftool is not as flexible would be false.\n\nI think the last one would be the way to go in the longer term.\n"},{"id":"128056","messageId":"200911210033.42661.markus.heidelberg@web.de","threadId":"21632","inReplyTo":"20091120185522.GC56351@gmail.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-11-20T23:33:42Z","receivedAt":"2009-11-20T23:33:42Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 2009-11-20:\n> The argument in favor of difftool is one of user\n> expectations. From a user's POV it ~seems~ desirable for\n> gitk to honor difftool configurations.\n\nIn general I absolutely like the idea. There are however problems you\ncan encounter.\nI use vimdiff for git-difftool and gvimdiff for gitk. When testing gitk\nwith your patch, I wondered where the editor is, because nothing popped\nup. I then found vim on the terminal from where I started gitk. Even\nworse: when started with \"gitk &\", the vim process runs somewhere in the\nbackground and gitk freezes.\n\nTo overcome this problem maybe we need a second config variable like\ndiff.guitool.\n\nMarkus\n"},{"id":"128084","messageId":"A2458D5A-DBAD-4F74-B166-92BEEB162418@gmail.com","threadId":"21632","inReplyTo":"7veinsx6lj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-11-21T21:47:17Z","receivedAt":"2009-11-21T21:47:17Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Nov 20, 2009, at 12:51 PM, Junio C Hamano wrote:\n\n> I do not read Tcl very well but I am guessing that in gitk you specify\n>>>\n>>\n> what tool to run (e.g. \"frobanodiff -z\"), gitk feeds you two temporary\n> files on the filesystem to compare (e.g. \"frobanodiff -z $tmp1 \n> $tmp2\"),\n> and your command line is responsible for giving satisfying diff \n> experience\n> to the end user.\n>\n> I see three possible approaches:\n>\n> * Teach \"git-difftool\" a mode to compare two arbitrary files on the\n> filesytem, and set that as \"External Diff\" command that takes the\n> filenames as extra two parameters, just like any other \"External \n> Diff\"\n> programs given to gitk does. This is the least palatable, as it \n> won't\n> solve the read-only repository issue at all (it only allows you the\n> logic to choose the configured difftool backend program).\n>\n> * Instead of disabling the traditional \"External Diff\" and taking it \n> over\n> like your patch did, add a new codepath for \"Difftool\" that feeds \n> the\n> commit IDs and paths the way git-difftool expects. The user can use\n> both, and the issue of read-only repository is solved when \n> \"Difftool\"\n> is used (but not \"External Diff\").\n>\n> * Take over \"External Diff\" codepath exactly like your patch did, but\n> teach \"git-difftool\" a new command line option to name an \n> unconfigured\n> external program that takes two filenames. When \"External Diff\"\n> program is *not* configured in gitk, the command line to invoke\n> difftool would be exactly as in your patch, i.e. \"difftool --no- \n> prompt\n> $from $to -- $path\". Otherwise, when gitk is configured to use an\n> external program, e.g. \"frobanodiff -z\", for \"External Diff\", you \n> pass\n> that command line to \"git-difftool\" via that new option, e.g.\n>\n> difftool --no-prompt --extcmd=\"frobanodiff -z\" $from $to -- $path\n>\n> Then difftool is responsible for preparing the two necessary \n> temporary\n> files out of the given information ($from/$to/$path) and feeding \n> them\n> to \"frobanodiff -z\" command line.\n>\n> Maybe such --extcmd support already exists in difftool, in which \n> case\n> my earlier suspicion that difftool is not as flexible would be \n> false.\n\nSounds good. Adding --extcmd should be nice and straightforward. \nMarkus mentioned the need for a diff.guitool variable that would be \ntested for in the gitk case so it sounds like having a --gui option to \nlet difftool know to do that should then cover all the bases.\n\nIt's looking like we'll have ourselves a small difftool patch series \nsoon. Thanks all,\n\n-- \n David \n \n"},{"id":"130510","messageId":"20091230121319.6117@nanako3.lavabit.com","threadId":"21632","inReplyTo":"1258341126-2108-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-30T03:13:19Z","receivedAt":"2009-12-30T03:13:19Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Junio, could you tell us what happened to this thread?\n"},{"id":"130523","messageId":"7vy6kk52an.fsf@alter.siamese.dyndns.org","threadId":"21632","inReplyTo":"20091230121319.6117@nanako3.lavabit.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-30T07:49:52Z","receivedAt":"2009-12-30T07:49:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Junio, could you tell us what happened to this thread?\n\nSee http://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133414\n\nIn short, the particular way to call difftool this patch implements was\nfound to be inadequate to support existing external diff support by gitk\nand a small difftool update will happen first, followed by a patch to gitk\nto use the updated difftool, to avoid regression.\n"},{"id":"130594","messageId":"20091231071642.GA10067@gmail.com","threadId":"21632","inReplyTo":"7vy6kk52an.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-12-31T07:16:45Z","receivedAt":"2009-12-31T07:16:45Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Tue, Dec 29, 2009 at 11:49:52PM -0800, Junio C Hamano wrote:\n> Nanako Shiraishi writes:\n> \n> > Junio, could you tell us what happened to this thread?\n> \n> See http://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133414\n> \n> In short, the particular way to call difftool this patch implements was\n> found to be inadequate to support existing external diff support by gitk\n> and a small difftool update will happen first, followed by a patch to gitk\n> to use the updated difftool, to avoid regression.\n\n\nI started the first step:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/135613\nhttp://thread.gmane.org/gmane.comp.version-control.git/135613/focus=135612\n\nThe 2nd patch implements the the --gui option which Markus\npointed out would be needed to avoid issues such as calling\n\"vimdiff\" from a console-less gitk:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/133386\n\n\nI marked the --gui patch as \"RFC\" since it introduced a new\nconfig variable and I want to make sure that we agreed on its\nname. I didn't get any feedback about that patch\n(my fault-- we were in RC freeze and I forgot to CC: Markus).\n\nIf that looks like a good first step then we can do the next\nstep which would be to introduce the --extcmd= option as\nmentioned here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133386\n\nI will try and prepare the changes for --extcmd= within\nthe next week assuming the existing --gui patch is ok.\n\n\nOn a related note, Jay Soffian recently submitted a\ngit-mergetool--lib patch adding support for \"diffmerge\".\nIt made it clear that there were parts of git-mergetool--lib\nthat could use some refactoring:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/134906\n\nJay did mention that he'd give it a shot at the time, though\nit does seems like the refactoring could wait until we see\nhow --extcmd= fits into the world.\n\n\nThank you for following up on this thread, Nanako.\n\n-- \n\t\tDavid\n"},{"id":"130631","messageId":"7vbpheswvo.fsf@alter.siamese.dyndns.org","threadId":"21632","inReplyTo":"20091231071642.GA10067@gmail.com","subject":"Re: [PATCH] gitk: Use git-difftool for external diffs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-31T20:37:31Z","receivedAt":"2009-12-31T20:37:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> I started the first step:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/135613\n> http://thread.gmane.org/gmane.comp.version-control.git/135613/focus=135612\n\nThanks.\n\n> The 2nd patch implements the the --gui option which Markus\n> pointed out would be needed to avoid issues such as calling\n> \"vimdiff\" from a console-less gitk:\n>\n> http://article.gmane.org/gmane.comp.version-control.git/133386\n>\n> I marked the --gui patch as \"RFC\" since it introduced a new\n> config variable and I want to make sure that we agreed on its\n> name. I didn't get any feedback about that patch\n> (my fault-- we were in RC freeze and I forgot to CC: Markus).\n\nI don't think \"diff.guitool\" would hurt. However,...\n\nI think the \"--gui\" patch is a more or less independent issue to \"gitk\nruns external diff through difftool\", because difftool/mergetool already\nhave a built-in way to auto-guess which backend to use depending on what\nits environment looks like (e.g. do we have $DISPLAY etc.).\n\nThe \"--gui\" patch is about giving a more explicit way for the caller to\ncontrol that backend picking decision process and it is more like icing\nthan a prerequisite for the issue. IOW, I think the end result will be\nusable by gitk users even if they do not configure \"diff.guitool\".\n"}]}