From: David Aguilar <hidden> Date: 2016-06-15 22:48:30
This teaches gitk about git-difftool. A benefit of this change
is that gitk's external diff feature now works with read-only
repositories.
Signed-off-by: David Aguilar <redacted>
---
This is still not the final result but it does get us
to a better place (having gitk work in read-only repos).
We may later want to add a radio button with "difftool"
as a choice so that the configured difftool is used
instead of the one specified being specified in --extcmd.
Original thread:
http://thread.gmane.org/gmane.comp.version-control.git/132983
An even older attempt to fix the tempdir problem:
http://thread.gmane.org/gmane.comp.version-control.git/133277
This diffstat alone still makes me happy.
gitk | 59 ++++++++++-------------------------------------------------
1 files changed, 10 insertions(+), 49 deletions(-)
From: David Aguilar <hidden> Date: 2016-06-15 22:48:30
This teaches gitk about git-difftool. A benefit of this change
is that gitk's external diff feature now works with read-only
repositories.
Signed-off-by: David Aguilar <redacted>
---
Differences since the first patch:
This one doesn't pass "--gui" to difftool.
"--gui" was mistakenly included in the first patch and
consequently uncovered a bug in difftool.
gitk | 58 +++++++++-------------------------------------------------
1 files changed, 9 insertions(+), 49 deletions(-)
From: David Aguilar <hidden> Date: 2016-06-15 22:48:30
This teaches gitk about git-difftool. A benefit of this change
is that gitk's external diff feature now works with read-only
repositories.
Signed-off-by: David Aguilar <redacted>
---
Differences since v1 and v2:
v1: Do not pass "--gui" to difftool
v2: Do not needlessly check if $flist_menu_file is non-empty
gitk | 58 ++++++++--------------------------------------------------
1 files changed, 8 insertions(+), 50 deletions(-)
From: David Aguilar <hidden> Date: 2016-06-15 22:48:32
This teaches gitk about git-difftool. A benefit of this change
is that gitk's external diff feature now works with read-only
repositories.
Signed-off-by: David Aguilar <redacted>
---
gitk | 58 ++++++++--------------------------------------------------
1 files changed, 8 insertions(+), 50 deletions(-)
From: Markus Heidelberg <hidden> Date: 2016-06-15 22:48:32
David Aguilar, 2010-03-31 04:09:
This teaches gitk about git-difftool. A benefit of this change
is that gitk's external diff feature now works with read-only
repositories.
Signed-off-by: David Aguilar <redacted>
From: David Aguilar <hidden> Date: 2016-06-15 22:48:35
On Fri, Apr 02, 2010 at 12:32:53PM +0100, Markus Heidelberg wrote:
David Aguilar, 2010-03-31 04:09:
quoted
This teaches gitk about git-difftool. A benefit of this change
is that gitk's external diff feature now works with read-only
repositories.
Signed-off-by: David Aguilar <redacted>
Tested-by: Markus Heidelberg <redacted>
Paul, you haven't replied yet but I do remember you mentioning
that you like to keep gitk backwards-compat with older git.
I have a newer version of this patch that checks for the
existence of difftool --extcmd=<cmd> before trying to use it.
It'll be PATCH v5.
--
David
From: David Aguilar <hidden> Date: 2016-06-15 22:48:35
git-difftool is used instead of the built-in external diff
code when git is >= 1.7.0. git-difftool's '--extcmd=frotz'
feature was first introduced in 1.7.0 and is the mechanism
through which we launch the configured difftool.
A benefit of this change is that gitk's external diff feature
no longer needs write-access to the current directory.
Signed-off-by: David Aguilar <redacted>
---
Unlike previous versions, this one checks the git version at runtime
and gracefully falls back to the old code when git is older.
gitk | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
@@ -3355,6 +3355,7 @@ proc external_diff {} { global flist_menu_file global diffids global extdifftool+ global git_version if {[llength $diffids] == 1} { # no reference commit given
@@ -3375,6 +3376,19 @@ proc external_diff {} { set diffidto [lindex $diffids 1] }+ if {[package vcompare $git_version "1.7.0"] >= 0} {+ set cmd [list "git" "difftool" "--no-prompt" "--extcmd=$extdifftool"]+ if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {+ lappend cmd $diffidfrom+ }+ if {$diffidto ne $nullid && $diffidto ne $nullid2} {+ lappend cmd $diffidto+ }+ lappend cmd "--" $flist_menu_file+ eval exec $cmd &+ return+ }+ # make sure that several diffs wont collide set diffdir [gitknewtmpdir] if {$diffdir eq {}} return
From: Paul Mackerras <hidden> Date: 2016-06-15 22:48:39
On Thu, Apr 08, 2010 at 02:08:10AM -0700, David Aguilar wrote:
git-difftool is used instead of the built-in external diff
code when git is >= 1.7.0. git-difftool's '--extcmd=frotz'
feature was first introduced in 1.7.0 and is the mechanism
through which we launch the configured difftool.
A benefit of this change is that gitk's external diff feature
no longer needs write-access to the current directory.
I applied this one, and in testing it I noticed that if the git
difftool invocation fails, for example because the user doesn't have
meld installed, there is no notification via the GUI. Instead an
error message is printed to stderr. We need to do something more like
what we do in the old-git case -- use open rather than exec -- and pop
up an error dialog with error_popup on error so that the user knows
what the problem is.
Another minor problem is that the file names that meld (or other diff
tool) displays are less informative now. For example I see
"KyaZ5d_gitk : 8iucke_gitk" as the tab label in meld instead of the
"[87d24ec87abc...e236e0833ff] gitk" label that we got before, which
was a little more informative, though the e236e0833ff is the end of
the second SHA1 rather than the beginning.
So I backed it out. The error handling needs to be fixed using
something like what delete_at_eof does (except that obviously we don't
have any files to delete).
Paul.
From: David Aguilar <hidden> Date: 2016-06-15 22:48:40
On Sat, Apr 17, 2010 at 06:52:30PM +1000, Paul Mackerras wrote:
On Thu, Apr 08, 2010 at 02:08:10AM -0700, David Aguilar wrote:
quoted
git-difftool is used instead of the built-in external diff
code when git is >= 1.7.0. git-difftool's '--extcmd=frotz'
feature was first introduced in 1.7.0 and is the mechanism
through which we launch the configured difftool.
A benefit of this change is that gitk's external diff feature
no longer needs write-access to the current directory.
I applied this one, and in testing it I noticed that if the git
difftool invocation fails, for example because the user doesn't have
meld installed, there is no notification via the GUI. Instead an
error message is printed to stderr. We need to do something more like
what we do in the old-git case -- use open rather than exec -- and pop
up an error dialog with error_popup on error so that the user knows
what the problem is.
Another minor problem is that the file names that meld (or other diff
tool) displays are less informative now. For example I see
"KyaZ5d_gitk : 8iucke_gitk" as the tab label in meld instead of the
"[87d24ec87abc...e236e0833ff] gitk" label that we got before, which
was a little more informative, though the e236e0833ff is the end of
the second SHA1 rather than the beginning.
So I backed it out. The error handling needs to be fixed using
something like what delete_at_eof does (except that obviously we don't
have any files to delete).
Paul.
I'll fix the error handling and add a few more notes to the
final commit message.
The title display is tool-specific so there's something we can
do about it on a tool-by-tool basis. This'll have to wait until
we can break out the tool-dependent parts of git-mergetool--lib
as was discussed in the past:
http://lists-archives.org/git/707440-mergetool-lib-add-diffmerge-as-a-pre-configured-mergetool-option.html
(sorry, couldn't find it on gmane... :-/)
Jay, you mentioned wanting to give Junio's approach a try as
well as looking into what mercurial did with mergetools.
Do you have any thoughts about it since then? I can help
factor out the backends if you don't think you'll have time to
get to it soon.
Once we factor out the backends we'll should have an easier
time working in additional variables for doing user-friendly
things like passing the --label= flag to meld.
git-difftool should be able to do it since its GIT_EXTERNAL_DIFF
helper is passed the sha1s.
Thanks all,
--
David
From: Jay Soffian <hidden> Date: 2016-06-15 22:48:40
On Sat, Apr 17, 2010 at 6:45 PM, David Aguilar [off-list ref] wrote:
Jay, you mentioned wanting to give Junio's approach a try as
well as looking into what mercurial did with mergetools.
Do you have any thoughts about it since then? I can help
factor out the backends if you don't think you'll have time to
get to it soon.
Wow, I need to stop starting new patches and start finishing some of
the ones on my todo list.
But sorry, no, I haven't had a chance to look at this and I'm not
likely to soon. So if you've got the time and inclination, I'm happy
for you to do the work. :-)
j.
From: David Aguilar <hidden> Date: 2016-06-15 22:48:40
git-difftool's '--extcmd=frotz' was added in 1.7.0 and
is the mechanism through which gitk launches the
configured 'extdifftool'. When 'extdifftool' is
misconfigured an error dialog is used to display
git-difftool's stdout and stderr.
The existing implementation moved into 'proc gitkextdiff'
for use with git < 1.7.0.
One benefit of this change is that gitk's external diff
no longer requires write-access to the current directory.
Signed-off-by: David Aguilar <redacted>
---
Changes since last time:
* Errors are shown using 'proc error_popup'
* The existing code moved into a tidy function
gitk | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
@@ -3361,6 +3361,7 @@ proc external_diff {} { global flist_menu_file global diffids global extdifftool+ global git_version if {[llength $diffids] == 1} { # no reference commit given
@@ -3380,6 +3381,30 @@ proc external_diff {} { set diffidfrom [lindex $diffids 0] set diffidto [lindex $diffids 1] }+ if {[package vcompare $git_version "1.7.0"] < 0} {+ gitkextdiff $diffidfrom $diffidto+ return+ }++ set cmd [list "git" "difftool" "--no-prompt" "--extcmd=$extdifftool"]+ if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {+ lappend cmd $diffidfrom+ }+ if {$diffidto ne $nullid && $diffidto ne $nullid2} {+ lappend cmd $diffidto+ }+ lappend cmd "--" $flist_menu_file++ set pipe [open |$cmd r]+ set stdout [read $pipe]+ if {[catch {close $pipe} stderr] != 0} {+ error_popup "git-difftool: $stdout $stderr"+ }+}++proc gitkextdiff {diffidfrom diffidto} {+ global flist_menu_file+ global extdifftool # make sure that several diffs wont collide set diffdir [gitknewtmpdir]
From: David Aguilar <hidden> Date: 2016-06-15 22:48:56
Hello,
Is there anything else (besides sending this email) that I can
do to help move this patch along?
I got another "gitk read-only repo broken" email this week,
which is what reminded me... ;-)
It's been a while. I just rebased the patch against the latest
master and it didn't have any conflicts. Resend?
On Tue, Apr 20, 2010 at 01:11:19AM -0700, David Aguilar wrote:
quoted hunk
git-difftool's '--extcmd=frotz' was added in 1.7.0 and
is the mechanism through which gitk launches the
configured 'extdifftool'. When 'extdifftool' is
misconfigured an error dialog is used to display
git-difftool's stdout and stderr.
The existing implementation moved into 'proc gitkextdiff'
for use with git < 1.7.0.
One benefit of this change is that gitk's external diff
no longer requires write-access to the current directory.
Signed-off-by: David Aguilar <redacted>
---
Changes since last time:
* Errors are shown using 'proc error_popup'
* The existing code moved into a tidy function
gitk | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
@@ -3361,6 +3361,7 @@ proc external_diff {} { global flist_menu_file global diffids global extdifftool+ global git_version if {[llength $diffids] == 1} { # no reference commit given
@@ -3380,6 +3381,30 @@ proc external_diff {} { set diffidfrom [lindex $diffids 0] set diffidto [lindex $diffids 1] }+ if {[package vcompare $git_version "1.7.0"] < 0} {+ gitkextdiff $diffidfrom $diffidto+ return+ }++ set cmd [list "git" "difftool" "--no-prompt" "--extcmd=$extdifftool"]+ if {$diffidfrom ne $nullid && $diffidfrom ne $nullid2} {+ lappend cmd $diffidfrom+ }+ if {$diffidto ne $nullid && $diffidto ne $nullid2} {+ lappend cmd $diffidto+ }+ lappend cmd "--" $flist_menu_file++ set pipe [open |$cmd r]+ set stdout [read $pipe]+ if {[catch {close $pipe} stderr] != 0} {+ error_popup "git-difftool: $stdout $stderr"+ }+}++proc gitkextdiff {diffidfrom diffidto} {+ global flist_menu_file+ global extdifftool # make sure that several diffs wont collide set diffdir [gitknewtmpdir]