{"thread":{"id":"23205","subject":"[PATCH] gitk: Use git-difftool for external diffs","startedAt":"2010-03-27T21:45:16Z","lastAt":"2010-06-08T08:10:42Z","messageCount":14,"participants":["David Aguilar","Markus Heidelberg","Paul Mackerras","Jay Soffian"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"137974","messageId":"1269726316-30443-1-git-send-email-davvid@gmail.com","threadId":"23205","inReplyTo":null,"subject":"[PATCH] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-03-27T21:45:16Z","receivedAt":"2010-03-27T21:45:16Z","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\nis that gitk's external diff feature now works with read-only\nrepositories.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n\nThis is still not the final result but it does get us\nto a better place (having gitk work in read-only repos).\n\nWe may later want to add a radio button with \"difftool\"\nas a choice so that the configured difftool is used\ninstead of the one specified being specified in --extcmd.\n\nOriginal thread:\nhttp://thread.gmane.org/gmane.comp.version-control.git/132983\n\nAn even older attempt to fix the tempdir problem:\nhttp://thread.gmane.org/gmane.comp.version-control.git/133277\n\nThis diffstat alone still makes me happy.\n\n gitk |   59 ++++++++++-------------------------------------------------\n 1 files changed, 10 insertions(+), 49 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1f36a3e..7e114da 100755\n--- a/gitk\n+++ b/gitk\n@@ -3317,39 +3317,6 @@ proc gitknewtmpdir {} {\n     return $diffdir\n }\n \n-proc save_file_from_commit {filename output what} {\n-    global nullfile\n-\n-    if {[catch {exec git show $filename -- > $output} err]} {\n-\tif {[string match \"fatal: bad revision *\" $err]} {\n-\t    return $nullfile\n-\t}\n-\terror_popup \"[mc \"Error getting \\\"%s\\\" from %s:\" $filename $what] $err\"\n-\treturn {}\n-    }\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@@ -3375,23 +3342,17 @@ 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+    if {$flist_menu_file ne {}} {\n+        set cmd [list \"git\" \"difftool\" \"--no-prompt\" \"--gui\"]\n+        lappend cmd \"--extcmd\" $extdifftool\n+        if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {\n+            lappend cmd $diffidfrom\n+        }\n+        if {$diffidto ne $nullid && $diffidto ne $nullid2} {\n+            lappend cmd $diffidto\n         }\n+        lappend cmd \"--\" $flist_menu_file\n+        eval exec $cmd &\n     }\n }\n \n-- \n1.7.0.3.291.g5e4f6\n"},{"id":"137982","messageId":"1269734510-11503-1-git-send-email-davvid@gmail.com","threadId":"23205","inReplyTo":"1269726316-30443-1-git-send-email-davvid@gmail.com","subject":"[PATCH v2] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-03-28T00:01:50Z","receivedAt":"2010-03-28T00:01:50Z","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\nis that gitk's external diff feature now works with read-only\nrepositories.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n\nDifferences since the first patch:\nThis one doesn't pass \"--gui\" to difftool.\n\n\"--gui\" was mistakenly included in the first patch and\nconsequently uncovered a bug in difftool.\n\n gitk |   58 +++++++++-------------------------------------------------\n 1 files changed, 9 insertions(+), 49 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1f36a3e..46c103e 100755\n--- a/gitk\n+++ b/gitk\n@@ -3317,39 +3317,6 @@ proc gitknewtmpdir {} {\n     return $diffdir\n }\n \n-proc save_file_from_commit {filename output what} {\n-    global nullfile\n-\n-    if {[catch {exec git show $filename -- > $output} err]} {\n-\tif {[string match \"fatal: bad revision *\" $err]} {\n-\t    return $nullfile\n-\t}\n-\terror_popup \"[mc \"Error getting \\\"%s\\\" from %s:\" $filename $what] $err\"\n-\treturn {}\n-    }\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@@ -3375,23 +3342,16 @@ 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+    if {$flist_menu_file ne {}} {\n+        set cmd [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd\" $extdifftool]\n+        if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {\n+            lappend cmd $diffidfrom\n+        }\n+        if {$diffidto ne $nullid && $diffidto ne $nullid2} {\n+            lappend cmd $diffidto\n         }\n+        lappend cmd \"--\" $flist_menu_file\n+        eval exec $cmd &\n     }\n }\n \n-- \n1.7.0.3.292.gbeff\n"},{"id":"137983","messageId":"1269735600-12769-1-git-send-email-davvid@gmail.com","threadId":"23205","inReplyTo":"1269726316-30443-1-git-send-email-davvid@gmail.com","subject":"[PATCH v3] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-03-28T00:20:00Z","receivedAt":"2010-03-28T00:20:00Z","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\nis that gitk's external diff feature now works with read-only\nrepositories.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n\nDifferences since v1 and v2:\n\nv1: Do not pass \"--gui\" to difftool\nv2: Do not needlessly check if $flist_menu_file is non-empty\n\n gitk |   58 ++++++++--------------------------------------------------\n 1 files changed, 8 insertions(+), 50 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1f36a3e..637f8f9 100755\n--- a/gitk\n+++ b/gitk\n@@ -3317,39 +3317,6 @@ proc gitknewtmpdir {} {\n     return $diffdir\n }\n \n-proc save_file_from_commit {filename output what} {\n-    global nullfile\n-\n-    if {[catch {exec git show $filename -- > $output} err]} {\n-\tif {[string match \"fatal: bad revision *\" $err]} {\n-\t    return $nullfile\n-\t}\n-\terror_popup \"[mc \"Error getting \\\"%s\\\" from %s:\" $filename $what] $err\"\n-\treturn {}\n-    }\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@@ -3375,24 +3342,15 @@ 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 [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd\" $extdifftool]\n+    if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {\n+        lappend cmd $diffidfrom\n+    }\n+    if {$diffidto ne $nullid && $diffidto ne $nullid2} {\n+        lappend cmd $diffidto\n     }\n+    lappend cmd \"--\" $flist_menu_file\n+    eval exec $cmd &\n }\n \n proc find_hunk_blamespec {base line} {\n-- \n1.7.0.3.292.gbeff\n"},{"id":"138000","messageId":"201003281259.06947.markus.heidelberg@web.de","threadId":"23205","inReplyTo":"1269735600-12769-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v3] gitk: Use git-difftool for external diffs","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-03-28T10:59:06Z","receivedAt":"2010-03-28T10:59:06Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 2010-03-28 01:20:\n> +    set cmd [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd\" $extdifftool]\n\nAccording to \"man gitcli\" this should be \"--extcmd=$extdifftool\".\n\nBesides this, works for me.\n\nMarkus\n"},{"id":"138247","messageId":"20100331020623.GB23545@gmail.com","threadId":"23205","inReplyTo":"201003281259.06947.markus.heidelberg@web.de","subject":"Re: [PATCH v3] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-03-31T02:06:24Z","receivedAt":"2010-03-31T02:06:24Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, Mar 28, 2010 at 11:59:06AM +0100, Markus Heidelberg wrote:\n> David Aguilar, 2010-03-28 01:20:\n> > +    set cmd [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd\" $extdifftool]\n> \n> According to \"man gitcli\" this should be \"--extcmd=$extdifftool\".\n> \n> Besides this, works for me.\n> \n> Markus\n\nI'll send a v4 with this change.\nThanks for the review.\n\n-- \n\t\tDavid\n"},{"id":"138248","messageId":"1270001399-26031-1-git-send-email-davvid@gmail.com","threadId":"23205","inReplyTo":"201003281259.06947.markus.heidelberg@web.de","subject":"[PATCH v4] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-03-31T02:09:59Z","receivedAt":"2010-03-31T02:09:59Z","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\nis that gitk's external diff feature now works with read-only\nrepositories.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n gitk |   58 ++++++++--------------------------------------------------\n 1 files changed, 8 insertions(+), 50 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1f36a3e..71cb501 100755\n--- a/gitk\n+++ b/gitk\n@@ -3317,39 +3317,6 @@ proc gitknewtmpdir {} {\n     return $diffdir\n }\n \n-proc save_file_from_commit {filename output what} {\n-    global nullfile\n-\n-    if {[catch {exec git show $filename -- > $output} err]} {\n-\tif {[string match \"fatal: bad revision *\" $err]} {\n-\t    return $nullfile\n-\t}\n-\terror_popup \"[mc \"Error getting \\\"%s\\\" from %s:\" $filename $what] $err\"\n-\treturn {}\n-    }\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@@ -3375,24 +3342,15 @@ 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 [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd=$extdifftool\"]\n+    if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {\n+        lappend cmd $diffidfrom\n+    }\n+    if {$diffidto ne $nullid && $diffidto ne $nullid2} {\n+        lappend cmd $diffidto\n     }\n+    lappend cmd \"--\" $flist_menu_file\n+    eval exec $cmd &\n }\n \n proc find_hunk_blamespec {base line} {\n-- \n1.7.0.3.313.g87b3c\n"},{"id":"138418","messageId":"201004021332.53562.markus.heidelberg@web.de","threadId":"23205","inReplyTo":"1270001399-26031-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v4] gitk: Use git-difftool for external diffs","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-02T11:32:53Z","receivedAt":"2010-04-02T11:32:53Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 2010-03-31 04:09:\n> This teaches gitk about git-difftool.  A benefit of this change\n> is that gitk's external diff feature now works with read-only\n> repositories.\n> \n> Signed-off-by: David Aguilar <davvid@gmail.com>\n\nTested-by: Markus Heidelberg <markus.heidelberg@web.de>\n"},{"id":"138954","messageId":"20100408090211.GA31594@gmail.com","threadId":"23205","inReplyTo":"201004021332.53562.markus.heidelberg@web.de","subject":"Re: [PATCH v4] gitk: Use git-difftool for external diffs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-04-08T09:02:14Z","receivedAt":"2010-04-08T09:02:14Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Fri, Apr 02, 2010 at 12:32:53PM +0100, Markus Heidelberg wrote:\n> David Aguilar, 2010-03-31 04:09:\n> > This teaches gitk about git-difftool.  A benefit of this change\n> > is that gitk's external diff feature now works with read-only\n> > repositories.\n> > \n> > Signed-off-by: David Aguilar <davvid@gmail.com>\n> \n> Tested-by: Markus Heidelberg <markus.heidelberg@web.de>\n\nPaul, you haven't replied yet but I do remember you mentioning\nthat you like to keep gitk backwards-compat with older git.\nI have a newer version of this patch that checks for the\nexistence of difftool --extcmd=<cmd> before trying to use it.\nIt'll be PATCH v5.\n\n-- \n\t\tDavid\n"},{"id":"138955","messageId":"1270717690-32133-1-git-send-email-davvid@gmail.com","threadId":"23205","inReplyTo":"20100408090211.GA31594@gmail.com","subject":"[PATCH v5] gitk: Use git-difftool for external diffs when git >= 1.7.0","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-04-08T09:08:10Z","receivedAt":"2010-04-08T09:08:10Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"git-difftool is used instead of the built-in external diff\ncode when git is >= 1.7.0.  git-difftool's '--extcmd=frotz'\nfeature was first introduced in 1.7.0 and is the mechanism\nthrough which we launch the configured difftool.\n\nA benefit of this change is that gitk's external diff feature\nno longer needs write-access to the current directory.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n\nUnlike previous versions, this one checks the git version at runtime\nand gracefully falls back to the old code when git is older.\n\n gitk |   14 ++++++++++++++\n 1 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1f36a3e..a49eecc 100755\n--- a/gitk\n+++ b/gitk\n@@ -3355,6 +3355,7 @@ proc external_diff {} {\n     global flist_menu_file\n     global diffids\n     global extdifftool\n+    global git_version\n \n     if {[llength $diffids] == 1} {\n         # no reference commit given\n@@ -3375,6 +3376,19 @@ proc external_diff {} {\n         set diffidto [lindex $diffids 1]\n     }\n \n+    if {[package vcompare $git_version \"1.7.0\"] >= 0} {\n+        set cmd [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd=$extdifftool\"]\n+        if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {\n+            lappend cmd $diffidfrom\n+        }\n+        if {$diffidto ne $nullid && $diffidto ne $nullid2} {\n+            lappend cmd $diffidto\n+        }\n+        lappend cmd \"--\" $flist_menu_file\n+        eval exec $cmd &\n+        return\n+    }\n+\n     # make sure that several diffs wont collide\n     set diffdir [gitknewtmpdir]\n     if {$diffdir eq {}} return\n-- \n1.7.0.3.313.g87b3c\n"},{"id":"139727","messageId":"20100417085230.GC6681@brick.ozlabs.ibm.com","threadId":"23205","inReplyTo":"1270717690-32133-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v5] gitk: Use git-difftool for external diffs when git >= 1.7.0","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2010-04-17T08:52:30Z","receivedAt":"2010-04-17T08:52:30Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Thu, Apr 08, 2010 at 02:08:10AM -0700, David Aguilar wrote:\n\n> git-difftool is used instead of the built-in external diff\n> code when git is >= 1.7.0.  git-difftool's '--extcmd=frotz'\n> feature was first introduced in 1.7.0 and is the mechanism\n> through which we launch the configured difftool.\n> \n> A benefit of this change is that gitk's external diff feature\n> no longer needs write-access to the current directory.\n\nI applied this one, and in testing it I noticed that if the git\ndifftool invocation fails, for example because the user doesn't have\nmeld installed, there is no notification via the GUI.  Instead an\nerror message is printed to stderr.  We need to do something more like\nwhat we do in the old-git case -- use open rather than exec -- and pop\nup an error dialog with error_popup on error so that the user knows\nwhat the problem is.\n\nAnother minor problem is that the file names that meld (or other diff\ntool) displays are less informative now.  For example I see\n\"KyaZ5d_gitk : 8iucke_gitk\" as the tab label in meld instead of the\n\"[87d24ec87abc...e236e0833ff] gitk\" label that we got before, which\nwas a little more informative, though the e236e0833ff is the end of\nthe second SHA1 rather than the beginning.\n\nSo I backed it out.  The error handling needs to be fixed using\nsomething like what delete_at_eof does (except that obviously we don't\nhave any files to delete).\n\nPaul.\n"},{"id":"139789","messageId":"20100417224556.GB9366@gmail.com","threadId":"23205","inReplyTo":"20100417085230.GC6681@brick.ozlabs.ibm.com","subject":"Re: [PATCH v5] gitk: Use git-difftool for external diffs when git >= 1.7.0","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-04-17T22:45:58Z","receivedAt":"2010-04-17T22:45:58Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, Apr 17, 2010 at 06:52:30PM +1000, Paul Mackerras wrote:\n> On Thu, Apr 08, 2010 at 02:08:10AM -0700, David Aguilar wrote:\n> \n> > git-difftool is used instead of the built-in external diff\n> > code when git is >= 1.7.0.  git-difftool's '--extcmd=frotz'\n> > feature was first introduced in 1.7.0 and is the mechanism\n> > through which we launch the configured difftool.\n> > \n> > A benefit of this change is that gitk's external diff feature\n> > no longer needs write-access to the current directory.\n> \n> I applied this one, and in testing it I noticed that if the git\n> difftool invocation fails, for example because the user doesn't have\n> meld installed, there is no notification via the GUI.  Instead an\n> error message is printed to stderr.  We need to do something more like\n> what we do in the old-git case -- use open rather than exec -- and pop\n> up an error dialog with error_popup on error so that the user knows\n> what the problem is.\n> \n> Another minor problem is that the file names that meld (or other diff\n> tool) displays are less informative now.  For example I see\n> \"KyaZ5d_gitk : 8iucke_gitk\" as the tab label in meld instead of the\n> \"[87d24ec87abc...e236e0833ff] gitk\" label that we got before, which\n> was a little more informative, though the e236e0833ff is the end of\n> the second SHA1 rather than the beginning.\n> \n> So I backed it out.  The error handling needs to be fixed using\n> something like what delete_at_eof does (except that obviously we don't\n> have any files to delete).\n> \n> Paul.\n\nI'll fix the error handling and add a few more notes to the\nfinal commit message.\n\nThe title display is tool-specific so there's something we can\ndo about it on a tool-by-tool basis.  This'll have to wait until\nwe can break out the tool-dependent parts of git-mergetool--lib\nas was discussed in the past:\n\nhttp://lists-archives.org/git/707440-mergetool-lib-add-diffmerge-as-a-pre-configured-mergetool-option.html\n(sorry, couldn't find it on gmane... :-/)\n\nJay, you mentioned wanting to give Junio's approach a try as\nwell as looking into what mercurial did with mergetools.\nDo you have any thoughts about it since then?  I can help\nfactor out the backends if you don't think you'll have time to\nget to it soon.\n\nOnce we factor out the backends we'll should have an easier\ntime working in additional variables for doing user-friendly\nthings like passing the --label= flag to meld.\ngit-difftool should be able to do it since its GIT_EXTERNAL_DIFF\nhelper is passed the sha1s.\n\nThanks all,\n\n-- \n\t\tDavid\n"},{"id":"139804","messageId":"l2t76718491004171920y80eba2d7lbeed0add15e6ffed@mail.gmail.com","threadId":"23205","inReplyTo":"20100417224556.GB9366@gmail.com","subject":"Re: [PATCH v5] gitk: Use git-difftool for external diffs when git >= 1.7.0","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-04-18T02:20:19Z","receivedAt":"2010-04-18T02:20:19Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, Apr 17, 2010 at 6:45 PM, David Aguilar <davvid@gmail.com> wrote:\n> Jay, you mentioned wanting to give Junio's approach a try as\n> well as looking into what mercurial did with mergetools.\n> Do you have any thoughts about it since then?  I can help\n> factor out the backends if you don't think you'll have time to\n> get to it soon.\n\nWow, I need to stop starting new patches and start finishing some of\nthe ones on my todo list.\n\nBut sorry, no, I haven't had a chance to look at this and I'm not\nlikely to soon. So if you've got the time and inclination, I'm happy\nfor you to do the work. :-)\n\nj.\n"},{"id":"139955","messageId":"1271751079-18884-1-git-send-email-davvid@gmail.com","threadId":"23205","inReplyTo":"20100417085230.GC6681@brick.ozlabs.ibm.com","subject":"[PATCH v6] gitk: Use git-difftool for external diffs when available","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-04-20T08:11:19Z","receivedAt":"2010-04-20T08:11:19Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"git-difftool's '--extcmd=frotz' was added in 1.7.0 and\nis the mechanism through which gitk launches the\nconfigured 'extdifftool'.  When 'extdifftool' is\nmisconfigured an error dialog is used to display\ngit-difftool's stdout and stderr.\n\nThe existing implementation moved into 'proc gitkextdiff'\nfor use with git < 1.7.0.\n\nOne benefit of this change is that gitk's external diff\nno longer requires write-access to the current directory.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n\nChanges since last time:\n\n* Errors are shown using 'proc error_popup'\n* The existing code moved into a tidy function\n\n gitk |   25 +++++++++++++++++++++++++\n 1 files changed, 25 insertions(+), 0 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1b0e09a..0533baf 100755\n--- a/gitk\n+++ b/gitk\n@@ -3361,6 +3361,7 @@ proc external_diff {} {\n     global flist_menu_file\n     global diffids\n     global extdifftool\n+    global git_version\n \n     if {[llength $diffids] == 1} {\n         # no reference commit given\n@@ -3380,6 +3381,30 @@ proc external_diff {} {\n         set diffidfrom [lindex $diffids 0]\n         set diffidto [lindex $diffids 1]\n     }\n+    if {[package vcompare $git_version \"1.7.0\"] < 0} {\n+        gitkextdiff $diffidfrom $diffidto\n+        return\n+    }\n+\n+    set cmd [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd=$extdifftool\"]\n+    if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {\n+        lappend cmd $diffidfrom\n+    }\n+    if {$diffidto ne $nullid && $diffidto ne $nullid2} {\n+        lappend cmd $diffidto\n+    }\n+    lappend cmd \"--\" $flist_menu_file\n+\n+    set pipe [open |$cmd r]\n+    set stdout [read $pipe]\n+    if {[catch {close $pipe} stderr] != 0} {\n+        error_popup \"git-difftool: $stdout $stderr\"\n+    }\n+}\n+\n+proc gitkextdiff {diffidfrom diffidto} {\n+    global flist_menu_file\n+    global extdifftool\n \n     # make sure that several diffs wont collide\n     set diffdir [gitknewtmpdir]\n-- \n1.7.1.rc2.5.gddd02\n"},{"id":"143214","messageId":"20100608081040.GB14366@gmail.com","threadId":"23205","inReplyTo":"1271751079-18884-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v6] gitk: Use git-difftool for external diffs when available","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2010-06-08T08:10:42Z","receivedAt":"2010-06-08T08:10:42Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Hello,\n\nIs there anything else (besides sending this email) that I can\ndo to help move this patch along?\n\nI got another \"gitk read-only repo broken\" email this week,\nwhich is what reminded me... ;-)\n\nIt's been a while.  I just rebased the patch against the latest\nmaster and it didn't have any conflicts.  Resend?\n\nOn Tue, Apr 20, 2010 at 01:11:19AM -0700, David Aguilar wrote:\n> git-difftool's '--extcmd=frotz' was added in 1.7.0 and\n> is the mechanism through which gitk launches the\n> configured 'extdifftool'.  When 'extdifftool' is\n> misconfigured an error dialog is used to display\n> git-difftool's stdout and stderr.\n> \n> The existing implementation moved into 'proc gitkextdiff'\n> for use with git < 1.7.0.\n> \n> One benefit of this change is that gitk's external diff\n> no longer requires write-access to the current directory.\n> \n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n> \n> Changes since last time:\n> \n> * Errors are shown using 'proc error_popup'\n> * The existing code moved into a tidy function\n> \n>  gitk |   25 +++++++++++++++++++++++++\n>  1 files changed, 25 insertions(+), 0 deletions(-)\n> \n> diff --git a/gitk b/gitk\n> index 1b0e09a..0533baf 100755\n> --- a/gitk\n> +++ b/gitk\n> @@ -3361,6 +3361,7 @@ proc external_diff {} {\n>      global flist_menu_file\n>      global diffids\n>      global extdifftool\n> +    global git_version\n>  \n>      if {[llength $diffids] == 1} {\n>          # no reference commit given\n> @@ -3380,6 +3381,30 @@ proc external_diff {} {\n>          set diffidfrom [lindex $diffids 0]\n>          set diffidto [lindex $diffids 1]\n>      }\n> +    if {[package vcompare $git_version \"1.7.0\"] < 0} {\n> +        gitkextdiff $diffidfrom $diffidto\n> +        return\n> +    }\n> +\n> +    set cmd [list \"git\" \"difftool\" \"--no-prompt\" \"--extcmd=$extdifftool\"]\n> +    if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {\n> +        lappend cmd $diffidfrom\n> +    }\n> +    if {$diffidto ne $nullid && $diffidto ne $nullid2} {\n> +        lappend cmd $diffidto\n> +    }\n> +    lappend cmd \"--\" $flist_menu_file\n> +\n> +    set pipe [open |$cmd r]\n> +    set stdout [read $pipe]\n> +    if {[catch {close $pipe} stderr] != 0} {\n> +        error_popup \"git-difftool: $stdout $stderr\"\n> +    }\n> +}\n> +\n> +proc gitkextdiff {diffidfrom diffidto} {\n> +    global flist_menu_file\n> +    global extdifftool\n>  \n>      # make sure that several diffs wont collide\n>      set diffdir [gitknewtmpdir]\n> -- \n> 1.7.1.rc2.5.gddd02\n> \n\n-- \n\t\tDavid\n"}]}