git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] git-gui: use textconv filter for diff and blame
@ 2010-07-28 12:23 Matthieu Moy
  2010-07-30  0:22 ` Pat Thoyts
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2010-07-28 12:23 UTC (permalink / raw)
  To: git
  Cc: Shawn O. Pearce, Clemens Buchacher, Clément Poulain,
	Diane Gasselin, Axel Bonnet, Matthieu Moy

From: Clément Poulain <clement.poulain@ensimag.imag.fr>

Create a checkbox "Use Textconv For Diffs and Blame" in git-gui options.
If checked and if the driver for the concerned file exists, git-gui calls diff
and blame with --textconv option

Signed-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>
Signed-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr>
Signed-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

This patch was originally written by Clément Poulain (a student of
mine), it was not mergeable at the time it was sent, since it relied
on a patch serie introducing "git cat-file --textconv". Now that this
cat-file --textconv is in a git release (1.7.2), I guess it's time to
get this merged into git-gui.

I did review and test the patch, but I'm mostly useless in TCL, so I
may have missed the obvious. That said, the patch is relatively simple
and looks OK.

 git-gui/git-gui.sh     |   28 +++++++++++++++++++++++++++-
 git-gui/lib/blame.tcl  |   21 +++++++++++++++++++--
 git-gui/lib/diff.tcl   |    5 ++++-
 git-gui/lib/option.tcl |    1 +
 4 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 7d54511..59edf39 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -269,6 +269,17 @@ proc is_config_true {name} {
 	}
 }
 
+proc is_config_false {name} {
+	global repo_config
+	if {[catch {set v $repo_config($name)}]} {
+		return 0
+	} elseif {$v eq {false} || $v eq {0} || $v eq {no}} {
+		return 1
+	} else {
+		return 0
+	}
+}
+
 proc get_config {name} {
 	global repo_config
 	if {[catch {set v $repo_config($name)}]} {
@@ -782,6 +793,7 @@ set default_config(user.email) {}
 
 set default_config(gui.encoding) [encoding system]
 set default_config(gui.matchtrackingbranch) false
+set default_config(gui.textconv) true
 set default_config(gui.pruneduringfetch) false
 set default_config(gui.trustmtime) false
 set default_config(gui.fastcopyblame) false
@@ -3405,6 +3417,19 @@ lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]
 $ctxmsm add separator
 create_common_diff_popup $ctxmsm
 
+proc has_textconv {path} {
+	if {[is_config_false gui.textconv]} {
+		return 0
+	}
+	set filter [gitattr $path diff set]
+	set textconv [get_config [join [list diff $filter textconv] .]]
+	if {$filter ne {set} && $textconv ne {}} {
+		return 1
+	} else {
+		return 0
+	}
+}
+
 proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
 	global current_diff_path file_states
 	set ::cursorX $x
@@ -3440,7 +3465,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {
 			|| {__} eq $state
 			|| {_O} eq $state
 			|| {_T} eq $state
-			|| {T_} eq $state} {
+			|| {T_} eq $state
+			|| [has_textconv $current_diff_path]} {
 			set s disabled
 		} else {
 			set s normal
diff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl
index 786b50b..ead68fd 100644
--- a/git-gui/lib/blame.tcl
+++ b/git-gui/lib/blame.tcl
@@ -449,11 +449,28 @@ method _load {jump} {
 
 	$status show [mc "Reading %s..." "$commit:[escape_path $path]"]
 	$w_path conf -text [escape_path $path]
+
+	set do_textconv 0
+	if {![is_config_false gui.textconv] && [git-version >= 1.7.2]} {
+		set filter [gitattr $path diff set]
+		set textconv [get_config [join [list diff $filter textconv] .]]
+		if {$filter ne {set} && $textconv ne {}} {
+			set do_textconv 1
+		}
+	}
 	if {$commit eq {}} {
-		set fd [open $path r]
+		if {$do_textconv ne 0} {
+			set fd [open "|$textconv $path" r]
+		} else {
+			set fd [open $path r]
+		}
 		fconfigure $fd -eofchar {}
 	} else {
-		set fd [git_read cat-file blob "$commit:$path"]
+		if {$do_textconv ne 0} {
+			set fd [git_read cat-file --textconv "$commit:$path"]
+		} else {
+			set fd [git_read cat-file blob "$commit:$path"]
+		}
 	}
 	fconfigure $fd \
 		-blocking 0 \
diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index ec8c11e..c628750 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl
@@ -55,7 +55,7 @@ proc handle_empty_diff {} {
 
 	set path $current_diff_path
 	set s $file_states($path)
-	if {[lindex $s 0] ne {_M}} return
+	if {[lindex $s 0] ne {_M} || [has_textconv $path]} return
 
 	# Prevent infinite rescan loops
 	incr diff_empty_count
@@ -280,6 +280,9 @@ proc start_show_diff {cont_info {add_opts {}}} {
 			lappend cmd diff-files
 		}
 	}
+	if {![is_config_false gui.textconv] && [git-version >= 1.6.1]} {
+		lappend cmd --textconv
+	}
 
 	if {[string match {160000 *} [lindex $s 2]]
 	 || [string match {160000 *} [lindex $s 3]]} {
diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index d4c5e45..3807c8d 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl
@@ -148,6 +148,7 @@ proc do_options {} {
 		{b gui.trustmtime  {mc "Trust File Modification Timestamps"}}
 		{b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}}
 		{b gui.matchtrackingbranch {mc "Match Tracking Branches"}}
+		{b gui.textconv {mc "Use Textconv For Diffs and Blames"}}
 		{b gui.fastcopyblame {mc "Blame Copy Only On Changed Files"}}
 		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
 		{i-0..300 gui.blamehistoryctx {mc "Blame History Context Radius (days)"}}
-- 
1.7.2.25.g9ebe3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: use textconv filter for diff and blame
  2010-07-28 12:23 [PATCH] git-gui: use textconv filter for diff and blame Matthieu Moy
@ 2010-07-30  0:22 ` Pat Thoyts
  2010-07-30  6:56   ` Matthieu Moy
  0 siblings, 1 reply; 6+ messages in thread
From: Pat Thoyts @ 2010-07-30  0:22 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: git, Shawn O. Pearce, Clemens Buchacher, Clément Poulain,
	Diane Gasselin, Axel Bonnet

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

>From: Clément Poulain <clement.poulain@ensimag.imag.fr>
>
>Create a checkbox "Use Textconv For Diffs and Blame" in git-gui options.
>If checked and if the driver for the concerned file exists, git-gui calls diff
>and blame with --textconv option
>
>Signed-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>
>Signed-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr>
>Signed-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr>
>Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>---
>
>This patch was originally written by Clément Poulain (a student of
>mine), it was not mergeable at the time it was sent, since it relied
>on a patch serie introducing "git cat-file --textconv". Now that this
>cat-file --textconv is in a git release (1.7.2), I guess it's time to
>get this merged into git-gui.
>
>I did review and test the patch, but I'm mostly useless in TCL, so I
>may have missed the obvious. That said, the patch is relatively simple
>and looks OK.

This looks generally fine but can you suggest some test that I can use
to ensure it is actually doing something. I tried committing some test
files containing cyrillic characters but I see no difference between
using an unpatched git-gui and your patched version with git 1.7.2

>diff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl
>index 786b50b..ead68fd 100644
>--- a/git-gui/lib/blame.tcl
>+++ b/git-gui/lib/blame.tcl
>@@ -449,11 +449,28 @@ method _load {jump} {
> 
> 	$status show [mc "Reading %s..." "$commit:[escape_path $path]"]
> 	$w_path conf -text [escape_path $path]
>+
>+	set do_textconv 0
>+	if {![is_config_false gui.textconv] && [git-version >= 1.7.2]} {
>+		set filter [gitattr $path diff set]
>+		set textconv [get_config [join [list diff $filter textconv] .]]
>+		if {$filter ne {set} && $textconv ne {}} {
>+			set do_textconv 1
>+		}
>+	}
> 	if {$commit eq {}} {
>-		set fd [open $path r]
>+		if {$do_textconv ne 0} {
>+			set fd [open "|$textconv $path" r]

This is better written as
  set fd [open |[list $textconv $path] r]
in case there are spaces in either of the two components.

>+		} else {
>+			set fd [open $path r]
>+		}
> 		fconfigure $fd -eofchar {}
> 	} else {
>-		set fd [git_read cat-file blob "$commit:$path"]
>+		if {$do_textconv ne 0} {
>+			set fd [git_read cat-file --textconv "$commit:$path"]
>+		} else {
>+			set fd [git_read cat-file blob "$commit:$path"]
>+		}
> 	}
> 	fconfigure $fd \
> 		-blocking 0 \

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: use textconv filter for diff and blame
  2010-07-30  0:22 ` Pat Thoyts
@ 2010-07-30  6:56   ` Matthieu Moy
  2010-07-30 10:30     ` Pat Thoyts
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2010-07-30  6:56 UTC (permalink / raw)
  To: Pat Thoyts
  Cc: git, Shawn O. Pearce, Clemens Buchacher, Clément Poulain,
	Diane Gasselin, Axel Bonnet

Pat Thoyts <patthoyts@users.sourceforge.net> writes:

> This looks generally fine but can you suggest some test that I can use
> to ensure it is actually doing something. I tried committing some test
> files containing cyrillic characters but I see no difference between
> using an unpatched git-gui and your patched version with git 1.7.2

textconv has not much to do with encoding. It's a way to tell git the
name of a command that converts a file into plain text. Typical usages
would be to use odt2txt, catdoc/antiword, or meta-information
extraction (like exiftags).

Have a look here:

  https://git.wiki.kernel.org/index.php/Textconv

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: use textconv filter for diff and blame
  2010-07-30  6:56   ` Matthieu Moy
@ 2010-07-30 10:30     ` Pat Thoyts
  2010-07-30 10:40       ` Matthieu Moy
  0 siblings, 1 reply; 6+ messages in thread
From: Pat Thoyts @ 2010-07-30 10:30 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: git, Shawn O. Pearce, Clemens Buchacher, Clément Poulain,
	Diane Gasselin, Axel Bonnet

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

>Pat Thoyts <patthoyts@users.sourceforge.net> writes:
>
>> This looks generally fine but can you suggest some test that I can use
>> to ensure it is actually doing something. I tried committing some test
>> files containing cyrillic characters but I see no difference between
>> using an unpatched git-gui and your patched version with git 1.7.2
>
>textconv has not much to do with encoding. It's a way to tell git the
>name of a command that converts a file into plain text. Typical usages
>would be to use odt2txt, catdoc/antiword, or meta-information
>extraction (like exiftags).
>
>Have a look here:
>
>  https://git.wiki.kernel.org/index.php/Textconv
>

OK thanks. This works very nicely once configured. I've applied it and
pushed this plus some outstanding git-gui patches from msysgit to the
git-gui repository at git://repo.or.cz/git-gui.git

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: use textconv filter for diff and blame
  2010-07-30 10:30     ` Pat Thoyts
@ 2010-07-30 10:40       ` Matthieu Moy
  2010-08-02 19:01         ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Matthieu Moy @ 2010-07-30 10:40 UTC (permalink / raw)
  To: Pat Thoyts
  Cc: git, Shawn O. Pearce, Junio C. Hamano, Clemens Buchacher,
	Clément Poulain, Diane Gasselin, Axel Bonnet

Pat Thoyts <patthoyts@users.sourceforge.net> writes:

> OK thanks. This works very nicely once configured. I've applied it and
> pushed this plus some outstanding git-gui patches from msysgit to the
> git-gui repository at git://repo.or.cz/git-gui.git

This is great: it also fixes the very long standing bug of "git gui
blame: Show History Context"
(http://mid.gmane.org/vpqzkxkorzr.fsf@bauges.imag.fr).

Junio: Can we get this merged into git.git?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: use textconv filter for diff and blame
  2010-07-30 10:40       ` Matthieu Moy
@ 2010-08-02 19:01         ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2010-08-02 19:01 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Pat Thoyts, git, Shawn O. Pearce, Clemens Buchacher,
	Clément Poulain, Diane Gasselin, Axel Bonnet

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> This is great: it also fixes the very long standing bug of "git gui
> blame: Show History Context"
> (http://mid.gmane.org/vpqzkxkorzr.fsf@bauges.imag.fr).
>
> Junio: Can we get this merged into git.git?

Will park in 'pu' for now; I'll double-check with Shawn how we/he would
want to proceed with git-gui in a longer term.

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-08-02 19:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-28 12:23 [PATCH] git-gui: use textconv filter for diff and blame Matthieu Moy
2010-07-30  0:22 ` Pat Thoyts
2010-07-30  6:56   ` Matthieu Moy
2010-07-30 10:30     ` Pat Thoyts
2010-07-30 10:40       ` Matthieu Moy
2010-08-02 19:01         ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).