{"thread":{"id":"24635","subject":"[BUG] git gui blame fails for multi-word textconv filter","startedAt":"2010-08-04T19:25:25Z","lastAt":"2010-08-19T08:05:06Z","messageCount":9,"participants":["Kirill Smelkov","Clément Poulain","Matthieu Moy","Pat Thoyts"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"147130","messageId":"20100804192525.GA13086@landau.phys.spbu.ru","threadId":"24635","inReplyTo":null,"subject":"[BUG] git gui blame fails for multi-word textconv filter","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2010-08-04T19:25:25Z","receivedAt":"2010-08-04T19:25:25Z","isPatch":false,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"Hello,\n\nI use\n\n [diff \"astextplain\"]\n textconv = run-mailcap --action=cat\n\nin my ~/.gitconfig, and this works for git `git blame` because of 41a457\nin git.git (textconv: use shell to run helper), but fails with git gui:\n\n $ git gui blame 21980.2--ИМС-МР231.doc\n Error in startup script: couldn't execute \"run-mailcap --action=cat\": no such file or directory\n while executing\n \"open |[list $textconv $path] r\"\n (procedure \"_load\" line 56)\n invoked from within\n \"_load $this $i_jump\"\n (procedure \"blame::new\" line 185)\n invoked from within\n \"blame::new $head $path $jump_spec\"\n (\"blame\" arm line 6)\n invoked from within\n \"switch -- $subcommand {\n browser {\n if {$jump_spec ne {}} usage\n if {$head eq {}} {\n if {$path ne {} && [file isdirectory $path]} {\n set head $...\"\n (\"blame\" arm line 57)\n invoked from within\n \"switch -- $subcommand {\n browser -\n blame {\n if {$subcommand eq \"blame\"} {\n set subcommand_args {[--line=] rev? path}\n } else {\n set subcommand_a...\"\n (file \"/home/kirr/local/git/libexec/git-core/git-gui\" line 2868)\n\n\n\nThats is maybe because we use `git cat-file --textconv` only for in .git\nentries, but since cat-file lacks support for work-tree git-gui calls\ntextconv filter itself manually on initial \"$commit eq {}\"?\n\nIf so, I'd better teach cat-file about worktree, instead of teaching\ngit-gui about running textconv filter through shell. Just a wish...\n\n\nThanks,\nKirill\n\n\nP.S. And thanks for finally collecting all those git-gui patches.\n"},{"id":"147144","messageId":"4C59FBD5.5090209@ensimag.imag.fr","threadId":"24635","inReplyTo":"20100804192525.GA13086@landau.phys.spbu.ru","subject":"Re: [BUG] git gui blame fails for multi-word textconv filter","fromName":"Clément Poulain","fromEmail":"clement.poulain@ensimag.imag.fr","sentAt":"2010-08-04T23:46:29Z","receivedAt":"2010-08-04T23:46:29Z","isPatch":false,"sender":{"key":"clement.poulain@ensimag.imag.fr","avatar":null},"body":"Le 04/08/2010 21:25, Kirill Smelkov a écrit :\n> Hello,\n>\n> I use\n>\n> [diff \"astextplain\"]\n> textconv = run-mailcap --action=cat\n>\n> in my ~/.gitconfig, and this works for git `git blame` because of 41a457\n> in git.git (textconv: use shell to run helper), but fails with git gui:\n>\n> $ git gui blame 21980.2--ИМС-МР231.doc\n> Error in startup script: couldn't execute \"run-mailcap --action=cat\": no such file or directory\n> \nI wonder if spaces can be the reason of this. Looks like Tcl is looking \nfor an executable called \"run-mailcap --action=cat\", and doesn't \ndistinguish path from options.\nI do not have much experience with Tcl, so I can't figure out how to \nsolve that. Some help would be appreciate :-)\n> while executing\n> \"open |[list $textconv $path] r\"\n> (procedure \"_load\" line 56)\n> invoked from within\n> \"_load $this $i_jump\"\n> (procedure \"blame::new\" line 185)\n> invoked from within\n> \"blame::new $head $path $jump_spec\"\n> (\"blame\" arm line 6)\n> invoked from within\n> \"switch -- $subcommand {\n> browser {\n> if {$jump_spec ne {}} usage\n> if {$head eq {}} {\n> if {$path ne {}&& [file isdirectory $path]} {\n> set head $...\"\n> (\"blame\" arm line 57)\n> invoked from within\n> \"switch -- $subcommand {\n> browser -\n> blame {\n> if {$subcommand eq \"blame\"} {\n> set subcommand_args {[--line=] rev? path}\n> } else {\n> set subcommand_a...\"\n> (file \"/home/kirr/local/git/libexec/git-core/git-gui\" line 2868)\n>\n>\n>\n> Thats is maybe because we use `git cat-file --textconv` only for in .git\n> entries, but since cat-file lacks support for work-tree git-gui calls\n> textconv filter itself manually on initial \"$commit eq {}\"?\n> \nYep, \"open |[list $textconv $path] r\" is the way we call textconv on the \nwork-tree copy of the concerned file.\n> If so, I'd better teach cat-file about worktree, instead of teaching\n> git-gui about running textconv filter through shell. Just a wish...\n> \nThis was discussed here: a1ace6b77167a2ad4b4995e8c4d09761@ensimag.fr , \nand your suggestion was considered ;-)\n"},{"id":"147161","messageId":"vpqlj8l2xd5.fsf@bauges.imag.fr","threadId":"24635","inReplyTo":"4C59FBD5.5090209@ensimag.imag.fr","subject":"Re: [BUG] git gui blame fails for multi-word textconv filter","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-05T09:59:50Z","receivedAt":"2010-08-05T09:59:50Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Clément Poulain writes:\n\n> I wonder if spaces can be the reason of this. Looks like Tcl is\n> looking for an executable called \"run-mailcap --action=cat\", and\n> doesn't distinguish path from options.\n\nYes, and it does this because we asked it to do so. Whitespace\ninterpretation is done by the shell, and there's no shell involved\nhere.\n\n> I do not have much experience with Tcl, so I can't figure out how to\n> solve that. Some help would be appreciate :-)\n\nPatch follows. It just runs the textconv using a shell, which does\nwhitespace interpretation (and more if needed). Tested with\n\ntextconv=odt2txt --width=40\n\nand a file containing whitespaces.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"147162","messageId":"1281002722-3042-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24635","inReplyTo":"vpqlj8l2xd5.fsf@bauges.imag.fr","subject":"[PATCH] git-gui: Use shell to launch textconv filter in \"blame\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-05T10:05:22Z","receivedAt":"2010-08-05T10:05:22Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This allows one to use textconv commands with arguments.\n\nSigned-off-by: Matthieu Moy \n---\n git-gui/Makefile | 1 +\n git-gui/git-gui.sh | 6 ++++++\n git-gui/lib/blame.tcl | 4 +++-\n 3 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/git-gui/Makefile b/git-gui/Makefile\nindex 197b55e..e22ba5c 100644\n--- a/git-gui/Makefile\n+++ b/git-gui/Makefile\n@@ -215,6 +215,7 @@ endif\n $(GITGUI_MAIN): git-gui.sh GIT-VERSION-FILE GIT-GUI-VARS\n \t$(QUIET_GEN)rm -f $@ $@+ && \\\n \tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+\t\t-e 's|@@SHELL_PATH@@|$(SHELL_PATH_SQ)|' \\\n \t\t-e '1,30s|^ argv0=$$0| argv0=$(GITGUI_SCRIPT)|' \\\n \t\t-e '1,30s|^ exec wish | exec '\\''$(TCLTK_PATH_SED)'\\'' |' \\\n \t\t-e 's/@@GITGUI_VERSION@@/$(GITGUI_VERSION)/g' \\\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex bb10489..9049abf 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -128,6 +128,7 @@ set _githtmldir {}\n set _reponame {}\n set _iscygwin {}\n set _search_path {}\n+set _shellpath {@@SHELL_PATH@@}\n \n set _trace [lsearch -exact $argv --trace]\n if {$_trace >= 0} {\n@@ -137,6 +138,11 @@ if {$_trace >= 0} {\n \tset _trace 0\n }\n \n+proc shellpath {} {\n+\tglobal _shellpath\n+\treturn $_shellpath\n+}\n+\n proc appname {} {\n \tglobal _appname\n \treturn $_appname\ndiff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl\nindex 2137ec9..77656d3 100644\n--- a/git-gui/lib/blame.tcl\n+++ b/git-gui/lib/blame.tcl\n@@ -460,7 +460,9 @@ method _load {jump} {\n \t}\n \tif {$commit eq {}} {\n \t\tif {$do_textconv ne 0} {\n-\t\t\tset fd [open |[list $textconv $path] r]\n+\t\t\t# Run textconv with sh -c \"...\" to allow it to\n+\t\t\t# contain command + arguments.\n+\t\t\tset fd [open |[list [shellpath] -c \"$textconv \\\"\\$0\\\"\" $path] r]\n \t\t} else {\n \t\t\tset fd [open $path r]\n \t\t}\n-- \n1.7.2.1.30.g18195\n"},{"id":"147269","messageId":"87iq3osm3h.fsf@fox.patthoyts.tk","threadId":"24635","inReplyTo":"4C59FBD5.5090209@ensimag.imag.fr","subject":"Re: [BUG] git gui blame fails for multi-word textconv filter","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2010-08-05T22:58:42Z","receivedAt":"2010-08-05T22:58:42Z","isPatch":false,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Clément Poulain writes:\n\n>Le 04/08/2010 21:25, Kirill Smelkov a écrit :\n>> Hello,\n>>\n>> I use\n>>\n>> [diff \"astextplain\"]\n>> textconv = run-mailcap --action=cat\n>>\n>> in my ~/.gitconfig, and this works for git `git blame` because of 41a457\n>> in git.git (textconv: use shell to run helper), but fails with git gui:\n>>\n>> $ git gui blame 21980.2--ИМС-МР231.doc\n>> Error in startup script: couldn't execute \"run-mailcap --action=cat\": no such file or directory\n>> \n>I wonder if spaces can be the reason of this. Looks like Tcl is\n>looking for an executable called \"run-mailcap --action=cat\", and\n>doesn't distinguish path from options.\n\nindeed. We passed a 2-element list and the first element is the\ncommand. To permit this we need to append the pathname to the command\ninstead.\n open |[linsert $path 0 $cmd_plus_args_list]\nis a safe way to do that.\n\nI should have thought of that too and not just repos with spaces in the path.\n\n-- \nPat Thoyts http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD\n"},{"id":"147270","messageId":"87aap0sljs.fsf@fox.patthoyts.tk","threadId":"24635","inReplyTo":"1281002722-3042-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] git-gui: Use shell to launch textconv filter in \"blame\"","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2010-08-05T23:10:31Z","receivedAt":"2010-08-05T23:10:31Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Matthieu Moy writes:\n\n>This allows one to use textconv commands with arguments.\n>\n>Signed-off-by: Matthieu Moy \n\n>diff --git a/git-gui/lib/blame.tcl b/git-gui/lib/blame.tcl\n>index 2137ec9..77656d3 100644\n>--- a/git-gui/lib/blame.tcl\n>+++ b/git-gui/lib/blame.tcl\n>@@ -460,7 +460,9 @@ method _load {jump} {\n> \t}\n> \tif {$commit eq {}} {\n> \t\tif {$do_textconv ne 0} {\n>-\t\t\tset fd [open |[list $textconv $path] r]\n>+\t\t\t# Run textconv with sh -c \"...\" to allow it to\n>+\t\t\t# contain command + arguments.\n>+\t\t\tset fd [open |[list [shellpath] -c \"$textconv \\\"\\$0\\\"\" $path] r]\n> \t\t} else {\n> \t\t\tset fd [open $path r]\n> \t\t}\n\nI don't believe we need to put all this in to launch this via the\nshell. We just have to pass a list where the first element is the\ncommand-name.\n\nThe following works for me using your 'textconv = odf2txt --width=40'\ntest and also a 'textconv = od -t x1' that I tried for a hex dump\noutput. I couldn't make run-mailcap do anything useful for me.\n\ndiff --git a/lib/blame.tcl b/lib/blame.tcl\nindex 2137ec9..c06ef04 100644\n--- a/lib/blame.tcl\n+++ b/lib/blame.tcl\n@@ -460,7 +460,7 @@ method _load {jump} {\n }\n if {$commit eq {}} {\n if {$do_textconv ne 0} {\n- set fd [open |[list $textconv $path] r]\n+ set fd [open |[linsert $textconv end $path] r]\n } else {\n set fd [open $path r]\n }\n\n-- \nPat Thoyts http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD\n"},{"id":"147304","messageId":"vpqaap0cees.fsf@bauges.imag.fr","threadId":"24635","inReplyTo":"87aap0sljs.fsf@fox.patthoyts.tk","subject":"Re: [PATCH] git-gui: Use shell to launch textconv filter in \"blame\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-06T08:51:23Z","receivedAt":"2010-08-06T08:51:23Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Pat Thoyts writes:\n\n>> \tif {$commit eq {}} {\n>> \t\tif {$do_textconv ne 0} {\n>>-\t\t\tset fd [open |[list $textconv $path] r]\n>>+\t\t\t# Run textconv with sh -c \"...\" to allow it to\n>>+\t\t\t# contain command + arguments.\n>>+\t\t\tset fd [open |[list [shellpath] -c \"$textconv \\\"\\$0\\\"\" $path] r]\n>> \t\t} else {\n>> \t\t\tset fd [open $path r]\n>> \t\t}\n>\n> I don't believe we need to put all this in to launch this via the\n> shell. We just have to pass a list where the first element is the\n> command-name.\n>\n> The following works for me using your 'textconv = odf2txt --width=40'\n> test and also a 'textconv = od -t x1' that I tried for a hex dump\n> output. I couldn't make run-mailcap do anything useful for me.\n>\n> diff --git a/lib/blame.tcl b/lib/blame.tcl\n> index 2137ec9..c06ef04 100644\n> --- a/lib/blame.tcl\n> +++ b/lib/blame.tcl\n> @@ -460,7 +460,7 @@ method _load {jump} {\n> }\n> if {$commit eq {}} {\n> if {$do_textconv ne 0} {\n> - set fd [open |[list $textconv $path] r]\n> + set fd [open |[linsert $textconv end $path] r]\n> } else {\n> set fd [open $path r]\n> }\n\nI'm not very fluent in Tcl, but I don't think this runs the command\nthrough a shell (pstree agrees with me). That will work in most cases,\nso that may be acceptable, but if you want to have full compatibility\nwith what \"git blame\" does (by using a shell) and allow e.g.\n\ntextconv = LANG=C some-command\n\nor\n\ntextconv = cd ../; do-whatever\n\nwhich are already managed by \"git blame\" and are OK with my version,\nit's not going to do it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"147331","messageId":"87eieby69a.fsf@fox.patthoyts.tk","threadId":"24635","inReplyTo":"vpqaap0cees.fsf@bauges.imag.fr","subject":"Re: [PATCH] git-gui: Use shell to launch textconv filter in \"blame\"","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2010-08-06T17:56:33Z","receivedAt":"2010-08-06T17:56:33Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Matthieu Moy writes:\n[snip]\n>\n>I'm not very fluent in Tcl, but I don't think this runs the command\n>through a shell (pstree agrees with me). That will work in most cases,\n>so that may be acceptable, but if you want to have full compatibility\n>with what \"git blame\" does (by using a shell) and allow e.g.\n>\n>textconv = LANG=C some-command\n>\n>or\n>\n>textconv = cd ../; do-whatever\n>\n>which are already managed by \"git blame\" and are OK with my version,\n>it's not going to do it.\n\nOK compatibility with 'git blame' is a valid reason to do it your\nway. Tcl's exec (or open| is equivalent) doesn't go via shell. I'll\ncheck this on windows and apply it. Thanks.\n\n-- \nPat Thoyts http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD\n"},{"id":"148437","messageId":"20100819080506.GA31474@landau.phys.spbu.ru","threadId":"24635","inReplyTo":"87iq3osm3h.fsf@fox.patthoyts.tk","subject":"Re: [BUG] git gui blame fails for multi-word textconv filter","fromName":"Kirill Smelkov","fromEmail":"kirr@landau.phys.spbu.ru","sentAt":"2010-08-19T08:05:06Z","receivedAt":"2010-08-19T08:05:06Z","isPatch":false,"sender":{"key":"kirr@navytux.spb.ru","avatar":"https://gravatar.com/avatar/cf3445fdad1849941e17ab25bf1ee7c5ea1be2deb444be25ff5f36b0e50a985f?d=mp&s=160"},"body":"On Thu, Aug 05, 2010 at 01:46:29AM +0200, Cl??ment Poulain wrote:\n> Le 04/08/2010 21:25, Kirill Smelkov a ??crit :\n> >Hello,\n> >\n> >I use\n> >\n> > [diff \"astextplain\"]\n> > textconv = run-mailcap --action=cat\n> >\n> >in my ~/.gitconfig, and this works for git `git blame` because of 41a457\n> >in git.git (textconv: use shell to run helper), but fails with git gui:\n> >\n> > $ git gui blame 21980.2--ИМС-МР231.doc\n> > Error in startup script: couldn't execute \"run-mailcap --action=cat\": \n> > no such file or directory\n\n[...]\n\n> >If so, I'd better teach cat-file about worktree, instead of teaching\n> >git-gui about running textconv filter through shell. Just a wish...\n> > \n> This was discussed here: a1ace6b77167a2ad4b4995e8c4d09761@ensimag.fr , \n> and your suggestion was considered ;-)\n\nI see, thanks. Sigh that it is done not that way, but anyway -\neverybody, thanks for fixing this.\n\nKirill\n"}]}