{"thread":{"id":"24554","subject":"[PATCH] git-gui: use textconv filter for diff and blame","startedAt":"2010-07-28T12:23:50Z","lastAt":"2010-08-02T19:01:31Z","messageCount":6,"participants":["Matthieu Moy","Pat Thoyts","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"146615","messageId":"1280319830-20483-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24554","inReplyTo":null,"subject":"[PATCH] git-gui: use textconv filter for diff and blame","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-28T12:23:50Z","receivedAt":"2010-07-28T12:23:50Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"From: Clément Poulain <clement.poulain@ensimag.imag.fr>\n\nCreate a checkbox \"Use Textconv For Diffs and Blame\" in git-gui options.\nIf checked and if the driver for the concerned file exists, git-gui calls diff\nand blame with --textconv option\n\nSigned-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>\nSigned-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr>\nSigned-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr>\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n\nThis patch was originally written by Clément Poulain (a student of\nmine), it was not mergeable at the time it was sent, since it relied\non a patch serie introducing \"git cat-file --textconv\". Now that this\ncat-file --textconv is in a git release (1.7.2), I guess it's time to\nget this merged into git-gui.\n\nI did review and test the patch, but I'm mostly useless in TCL, so I\nmay have missed the obvious. That said, the patch is relatively simple\nand looks OK.\n\n git-gui/git-gui.sh     |   28 +++++++++++++++++++++++++++-\n git-gui/lib/blame.tcl  |   21 +++++++++++++++++++--\n git-gui/lib/diff.tcl   |    5 ++++-\n git-gui/lib/option.tcl |    1 +\n 4 files changed, 51 insertions(+), 4 deletions(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 7d54511..59edf39 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -269,6 +269,17 @@ proc is_config_true {name} {\n \t}\n }\n \n+proc is_config_false {name} {\n+\tglobal repo_config\n+\tif {[catch {set v $repo_config($name)}]} {\n+\t\treturn 0\n+\t} elseif {$v eq {false} || $v eq {0} || $v eq {no}} {\n+\t\treturn 1\n+\t} else {\n+\t\treturn 0\n+\t}\n+}\n+\n proc get_config {name} {\n \tglobal repo_config\n \tif {[catch {set v $repo_config($name)}]} {\n@@ -782,6 +793,7 @@ set default_config(user.email) {}\n \n set default_config(gui.encoding) [encoding system]\n set default_config(gui.matchtrackingbranch) false\n+set default_config(gui.textconv) true\n set default_config(gui.pruneduringfetch) false\n set default_config(gui.trustmtime) false\n set default_config(gui.fastcopyblame) false\n@@ -3405,6 +3417,19 @@ lappend diff_actions [list $ctxmsm entryconf [$ctxmsm index last] -state]\n $ctxmsm add separator\n create_common_diff_popup $ctxmsm\n \n+proc has_textconv {path} {\n+\tif {[is_config_false gui.textconv]} {\n+\t\treturn 0\n+\t}\n+\tset filter [gitattr $path diff set]\n+\tset textconv [get_config [join [list diff $filter textconv] .]]\n+\tif {$filter ne {set} && $textconv ne {}} {\n+\t\treturn 1\n+\t} else {\n+\t\treturn 0\n+\t}\n+}\n+\n proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {\n \tglobal current_diff_path file_states\n \tset ::cursorX $x\n@@ -3440,7 +3465,8 @@ proc popup_diff_menu {ctxm ctxmmg ctxmsm x y X Y} {\n \t\t\t|| {__} eq $state\n \t\t\t|| {_O} eq $state\n \t\t\t|| {_T} eq $state\n-\t\t\t|| {T_} eq $state} {\n+\t\t\t|| {T_} eq $state\n+\t\t\t|| [has_textconv $current_diff_path]} {\n \t\t\tset s disabled\n \t\t} else {\n \t\t\tset s normal\ndiff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl\nindex 786b50b..ead68fd 100644\n--- a/git-gui/lib/blame.tcl\n+++ b/git-gui/lib/blame.tcl\n@@ -449,11 +449,28 @@ method _load {jump} {\n \n \t$status show [mc \"Reading %s...\" \"$commit:[escape_path $path]\"]\n \t$w_path conf -text [escape_path $path]\n+\n+\tset do_textconv 0\n+\tif {![is_config_false gui.textconv] && [git-version >= 1.7.2]} {\n+\t\tset filter [gitattr $path diff set]\n+\t\tset textconv [get_config [join [list diff $filter textconv] .]]\n+\t\tif {$filter ne {set} && $textconv ne {}} {\n+\t\t\tset do_textconv 1\n+\t\t}\n+\t}\n \tif {$commit eq {}} {\n-\t\tset fd [open $path r]\n+\t\tif {$do_textconv ne 0} {\n+\t\t\tset fd [open \"|$textconv $path\" r]\n+\t\t} else {\n+\t\t\tset fd [open $path r]\n+\t\t}\n \t\tfconfigure $fd -eofchar {}\n \t} else {\n-\t\tset fd [git_read cat-file blob \"$commit:$path\"]\n+\t\tif {$do_textconv ne 0} {\n+\t\t\tset fd [git_read cat-file --textconv \"$commit:$path\"]\n+\t\t} else {\n+\t\t\tset fd [git_read cat-file blob \"$commit:$path\"]\n+\t\t}\n \t}\n \tfconfigure $fd \\\n \t\t-blocking 0 \\\ndiff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl\nindex ec8c11e..c628750 100644\n--- a/git-gui/lib/diff.tcl\n+++ b/git-gui/lib/diff.tcl\n@@ -55,7 +55,7 @@ proc handle_empty_diff {} {\n \n \tset path $current_diff_path\n \tset s $file_states($path)\n-\tif {[lindex $s 0] ne {_M}} return\n+\tif {[lindex $s 0] ne {_M} || [has_textconv $path]} return\n \n \t# Prevent infinite rescan loops\n \tincr diff_empty_count\n@@ -280,6 +280,9 @@ proc start_show_diff {cont_info {add_opts {}}} {\n \t\t\tlappend cmd diff-files\n \t\t}\n \t}\n+\tif {![is_config_false gui.textconv] && [git-version >= 1.6.1]} {\n+\t\tlappend cmd --textconv\n+\t}\n \n \tif {[string match {160000 *} [lindex $s 2]]\n \t || [string match {160000 *} [lindex $s 3]]} {\ndiff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl\nindex d4c5e45..3807c8d 100644\n--- a/git-gui/lib/option.tcl\n+++ b/git-gui/lib/option.tcl\n@@ -148,6 +148,7 @@ proc do_options {} {\n \t\t{b gui.trustmtime  {mc \"Trust File Modification Timestamps\"}}\n \t\t{b gui.pruneduringfetch {mc \"Prune Tracking Branches During Fetch\"}}\n \t\t{b gui.matchtrackingbranch {mc \"Match Tracking Branches\"}}\n+\t\t{b gui.textconv {mc \"Use Textconv For Diffs and Blames\"}}\n \t\t{b gui.fastcopyblame {mc \"Blame Copy Only On Changed Files\"}}\n \t\t{i-20..200 gui.copyblamethreshold {mc \"Minimum Letters To Blame Copy On\"}}\n \t\t{i-0..300 gui.blamehistoryctx {mc \"Blame History Context Radius (days)\"}}\n-- \n1.7.2.25.g9ebe3\n"},{"id":"146748","messageId":"871valstsi.fsf@fox.patthoyts.tk","threadId":"24554","inReplyTo":"1280319830-20483-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] git-gui: use textconv filter for diff and blame","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2010-07-30T00:22:05Z","receivedAt":"2010-07-30T00:22:05Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n>From: Clément Poulain <clement.poulain@ensimag.imag.fr>\n>\n>Create a checkbox \"Use Textconv For Diffs and Blame\" in git-gui options.\n>If checked and if the driver for the concerned file exists, git-gui calls diff\n>and blame with --textconv option\n>\n>Signed-off-by: Clément Poulain <clement.poulain@ensimag.imag.fr>\n>Signed-off-by: Diane Gasselin <diane.gasselin@ensimag.imag.fr>\n>Signed-off-by: Axel Bonnet <axel.bonnet@ensimag.imag.fr>\n>Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n>---\n>\n>This patch was originally written by Clément Poulain (a student of\n>mine), it was not mergeable at the time it was sent, since it relied\n>on a patch serie introducing \"git cat-file --textconv\". Now that this\n>cat-file --textconv is in a git release (1.7.2), I guess it's time to\n>get this merged into git-gui.\n>\n>I did review and test the patch, but I'm mostly useless in TCL, so I\n>may have missed the obvious. That said, the patch is relatively simple\n>and looks OK.\n\nThis looks generally fine but can you suggest some test that I can use\nto ensure it is actually doing something. I tried committing some test\nfiles containing cyrillic characters but I see no difference between\nusing an unpatched git-gui and your patched version with git 1.7.2\n\n>diff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl\n>index 786b50b..ead68fd 100644\n>--- a/git-gui/lib/blame.tcl\n>+++ b/git-gui/lib/blame.tcl\n>@@ -449,11 +449,28 @@ method _load {jump} {\n> \n> \t$status show [mc \"Reading %s...\" \"$commit:[escape_path $path]\"]\n> \t$w_path conf -text [escape_path $path]\n>+\n>+\tset do_textconv 0\n>+\tif {![is_config_false gui.textconv] && [git-version >= 1.7.2]} {\n>+\t\tset filter [gitattr $path diff set]\n>+\t\tset textconv [get_config [join [list diff $filter textconv] .]]\n>+\t\tif {$filter ne {set} && $textconv ne {}} {\n>+\t\t\tset do_textconv 1\n>+\t\t}\n>+\t}\n> \tif {$commit eq {}} {\n>-\t\tset fd [open $path r]\n>+\t\tif {$do_textconv ne 0} {\n>+\t\t\tset fd [open \"|$textconv $path\" r]\n\nThis is better written as\n  set fd [open |[list $textconv $path] r]\nin case there are spaces in either of the two components.\n\n>+\t\t} else {\n>+\t\t\tset fd [open $path r]\n>+\t\t}\n> \t\tfconfigure $fd -eofchar {}\n> \t} else {\n>-\t\tset fd [git_read cat-file blob \"$commit:$path\"]\n>+\t\tif {$do_textconv ne 0} {\n>+\t\t\tset fd [git_read cat-file --textconv \"$commit:$path\"]\n>+\t\t} else {\n>+\t\t\tset fd [git_read cat-file blob \"$commit:$path\"]\n>+\t\t}\n> \t}\n> \tfconfigure $fd \\\n> \t\t-blocking 0 \\\n"},{"id":"146756","messageId":"vpq8w4tpie2.fsf@bauges.imag.fr","threadId":"24554","inReplyTo":"871valstsi.fsf@fox.patthoyts.tk","subject":"Re: [PATCH] git-gui: use textconv filter for diff and blame","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-30T06:56:37Z","receivedAt":"2010-07-30T06:56:37Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Pat Thoyts <patthoyts@users.sourceforge.net> writes:\n\n> This looks generally fine but can you suggest some test that I can use\n> to ensure it is actually doing something. I tried committing some test\n> files containing cyrillic characters but I see no difference between\n> using an unpatched git-gui and your patched version with git 1.7.2\n\ntextconv has not much to do with encoding. It's a way to tell git the\nname of a command that converts a file into plain text. Typical usages\nwould be to use odt2txt, catdoc/antiword, or meta-information\nextraction (like exiftags).\n\nHave a look here:\n\n  https://git.wiki.kernel.org/index.php/Textconv\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146773","messageId":"878w4tz2hi.fsf@fox.patthoyts.tk","threadId":"24554","inReplyTo":"vpq8w4tpie2.fsf@bauges.imag.fr","subject":"Re: [PATCH] git-gui: use textconv filter for diff and blame","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2010-07-30T10:30:01Z","receivedAt":"2010-07-30T10:30:01Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n>Pat Thoyts <patthoyts@users.sourceforge.net> writes:\n>\n>> This looks generally fine but can you suggest some test that I can use\n>> to ensure it is actually doing something. I tried committing some test\n>> files containing cyrillic characters but I see no difference between\n>> using an unpatched git-gui and your patched version with git 1.7.2\n>\n>textconv has not much to do with encoding. It's a way to tell git the\n>name of a command that converts a file into plain text. Typical usages\n>would be to use odt2txt, catdoc/antiword, or meta-information\n>extraction (like exiftags).\n>\n>Have a look here:\n>\n>  https://git.wiki.kernel.org/index.php/Textconv\n>\n\nOK thanks. This works very nicely once configured. I've applied it and\npushed this plus some outstanding git-gui patches from msysgit to the\ngit-gui repository at git://repo.or.cz/git-gui.git\n"},{"id":"146774","messageId":"vpqocdpdzgt.fsf@bauges.imag.fr","threadId":"24554","inReplyTo":"878w4tz2hi.fsf@fox.patthoyts.tk","subject":"Re: [PATCH] git-gui: use textconv filter for diff and blame","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-30T10:40:50Z","receivedAt":"2010-07-30T10:40:50Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Pat Thoyts <patthoyts@users.sourceforge.net> writes:\n\n> OK thanks. This works very nicely once configured. I've applied it and\n> pushed this plus some outstanding git-gui patches from msysgit to the\n> git-gui repository at git://repo.or.cz/git-gui.git\n\nThis is great: it also fixes the very long standing bug of \"git gui\nblame: Show History Context\"\n(http://mid.gmane.org/vpqzkxkorzr.fsf@bauges.imag.fr).\n\nJunio: Can we get this merged into git.git?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146972","messageId":"7vhbjcltys.fsf@alter.siamese.dyndns.org","threadId":"24554","inReplyTo":"vpqocdpdzgt.fsf@bauges.imag.fr","subject":"Re: [PATCH] git-gui: use textconv filter for diff and blame","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-02T19:01:31Z","receivedAt":"2010-08-02T19:01:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> This is great: it also fixes the very long standing bug of \"git gui\n> blame: Show History Context\"\n> (http://mid.gmane.org/vpqzkxkorzr.fsf@bauges.imag.fr).\n>\n> Junio: Can we get this merged into git.git?\n\nWill park in 'pu' for now; I'll double-check with Shawn how we/he would\nwant to proceed with git-gui in a longer term.\n\nThanks.\n"}]}