# [PATCH] git-gui: use textconv filter for diff and blame 6 messages from 2010-07-28 to 2010-08-02. Participants: Matthieu Moy, Pat Thoyts, Junio C Hamano. Thread: https://gitlist.dev/t/24554 ## Matthieu Moy, 2010-07-28 12:23 Subject: [PATCH] git-gui: use textconv filter for diff and blame Message-ID: <1280319830-20483-1-git-send-email-Matthieu.Moy@imag.fr> URL: https://gitlist.dev/e/1280319830-20483-1-git-send-email-Matthieu.Moy%40imag.fr ``` From: Clément Poulain 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 Signed-off-by: Diane Gasselin Signed-off-by: Axel Bonnet Signed-off-by: Matthieu Moy --- 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 ``` ## Pat Thoyts, 2010-07-30 00:22 Subject: Re: [PATCH] git-gui: use textconv filter for diff and blame Message-ID: <871valstsi.fsf@fox.patthoyts.tk> URL: https://gitlist.dev/e/871valstsi.fsf%40fox.patthoyts.tk In-Reply-To: <1280319830-20483-1-git-send-email-Matthieu.Moy@imag.fr> ``` Matthieu Moy writes: >From: Clément Poulain > >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 >Signed-off-by: Diane Gasselin >Signed-off-by: Axel Bonnet >Signed-off-by: Matthieu Moy >--- > >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 \ ``` ## Matthieu Moy, 2010-07-30 06:56 Subject: Re: [PATCH] git-gui: use textconv filter for diff and blame Message-ID: URL: https://gitlist.dev/e/vpq8w4tpie2.fsf%40bauges.imag.fr In-Reply-To: <871valstsi.fsf@fox.patthoyts.tk> ``` Pat Thoyts 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/ ``` ## Pat Thoyts, 2010-07-30 10:30 Subject: Re: [PATCH] git-gui: use textconv filter for diff and blame Message-ID: <878w4tz2hi.fsf@fox.patthoyts.tk> URL: https://gitlist.dev/e/878w4tz2hi.fsf%40fox.patthoyts.tk In-Reply-To: ``` Matthieu Moy writes: >Pat Thoyts 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 ``` ## Matthieu Moy, 2010-07-30 10:40 Subject: Re: [PATCH] git-gui: use textconv filter for diff and blame Message-ID: URL: https://gitlist.dev/e/vpqocdpdzgt.fsf%40bauges.imag.fr In-Reply-To: <878w4tz2hi.fsf@fox.patthoyts.tk> ``` Pat Thoyts 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/ ``` ## Junio C Hamano, 2010-08-02 19:01 Subject: Re: [PATCH] git-gui: use textconv filter for diff and blame Message-ID: <7vhbjcltys.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhbjcltys.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Matthieu Moy 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. ```