{"thread":{"id":"17805","subject":"\"add -p\" + filenames with UTF-8 multibyte characters = \"No changes\"","startedAt":"2009-02-15T18:40:11Z","lastAt":"2009-02-17T08:09:17Z","messageCount":8,"participants":["Antonio García Domínguez","Teemu Likonen","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"104796","messageId":"2b8265360902151040t49711942udd4862cc9df01da5@mail.gmail.com","threadId":"17805","inReplyTo":null,"subject":"\"add -p\" + filenames with UTF-8 multibyte characters = \"No changes\"","fromName":"Antonio García Domínguez","fromEmail":"nyoescape@gmail.com","sentAt":"2009-02-15T18:40:11Z","receivedAt":"2009-02-15T18:40:11Z","isPatch":false,"sender":{"key":"nyoescape@gmail.com","avatar":null},"body":"Hello all,\n\nI seem to have run into a bug in \"add -p\" and \"add -i\" when trying to\nstage diff hunks in tracked files with UTF-8 multibyte characters\n(such as \"á\"). If I add \"á\", commit, then modify it and try to run\n\"add -p\" on it, Git reports \"No changes\". \"add -i\" doesn't do\nanything, either.\n\nI've switched to 1.6.2.rc0.90.g0753 and the problem persists. If it\nhelps, I've attached a small shell script with a minimal recipe for\ntriggering the bug.\n\nThis should incorrectly report \"No changes\":\n./test-accents.sh\n\nAnd this should work fine:\nFILE=a ./test-accents.sh\n\n-Antonio\n"},{"id":"104798","messageId":"87tz6vr0g4.fsf@iki.fi","threadId":"17805","inReplyTo":"2b8265360902151040t49711942udd4862cc9df01da5@mail.gmail.com","subject":"Re: \"add -p\" + filenames with UTF-8 multibyte characters = \"No changes\"","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2009-02-15T18:59:07Z","receivedAt":"2009-02-15T18:59:07Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"On 2009-02-15 19:40 (+0100), Antonio García Domínguez wrote:\n\n> I seem to have run into a bug in \"add -p\" and \"add -i\" when trying to\n> stage diff hunks in tracked files with UTF-8 multibyte characters\n> (such as \"á\"). If I add \"á\", commit, then modify it and try to run\n> \"add -p\" on it, Git reports \"No changes\". \"add -i\" doesn't do\n> anything, either.\n>\n> I've switched to 1.6.2.rc0.90.g0753 and the problem persists. If it\n> helps, I've attached a small shell script with a minimal recipe for\n> triggering the bug.\n\nThis bug is documented in BUGS section of \"git add\" manual (see \"git\nhelp add\"). You can work it around with\n\n    git config --global core.quotepath false\n"},{"id":"104799","messageId":"87prhjqzwb.fsf@iki.fi","threadId":"17805","inReplyTo":"2b8265360902151100n2eca0182odf9543c1dd8a7f98@mail.gmail.com","subject":"Re: \"add -p\" + filenames with UTF-8 multibyte characters = \"No changes\"","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2009-02-15T19:11:00Z","receivedAt":"2009-02-15T19:11:00Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"[Added the Git list back to delivery.]\n\nOn 2009-02-15 20:00 (+0100), Antonio García Domínguez wrote:\n\n>> This bug is documented in BUGS section of \"git add\" manual (see \"git\n>> help add\"). You can work it around with\n>>\n>>    git config --global core.quotepath false\n>\n> Oh, sorry, I should have known better and RTFM. :-D\n\nNo problem at all. I was bitten by the same bug earlier, and as I wasn't\nable to fix it I sent a documentation patch instead. :-)\n\n\"core.quotepath=false\" is good for other purposes too. It prints UTF-8\nfilenames in diff headers in form that is actually readable. I think it\nwould be better default.\n"},{"id":"104892","messageId":"20090216033634.GA12461@coredump.intra.peff.net","threadId":"17805","inReplyTo":"87prhjqzwb.fsf@iki.fi","subject":"Re: \"add -p\" + filenames with UTF-8 multibyte characters = \"No changes\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-16T03:36:34Z","receivedAt":"2009-02-16T03:36:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2009 at 09:11:00PM +0200, Teemu Likonen wrote:\n\n> \"core.quotepath=false\" is good for other purposes too. It prints UTF-8\n> filenames in diff headers in form that is actually readable. I think it\n> would be better default.\n\nI am not opposed to setting this as a default, but I think there may be\nsome encoding issues to be dealt with. At the very least, format-patch\ngenerates messages without a content-type header. E.g.,:\n\n  $ touch föö && git add . && git commit -m one\n  $ echo content >föö && git commit -a -m two\n\n  $ git format-patch --stdout HEAD^ | sed '/^$/q'\n\n     vs\n\n  $ git config core.quotepath false\n  $ git format-patch --stdout HEAD^ | sed '/^$/q'\n\nSo now we have non-ascii in our email, but no header specifying\nencoding. Previous experience has shown that intermediate MTAs (like\nvger) will add their own header with whatever encoding they think is\nsensible (in the case of vger, iso8859-1), corrupting the mail if they\nguess wrong.\n\nBut what is the right encoding to specify? We can guess that it is\nwhatever the commit message is in (defaulting to utf-8). It is by no\nmeans correct, but it would probably work pretty well in practice.\n\nOn the other hand, we already have the same problem for encoded file\n_contents_. So maybe it is not a big problem in practice.\n\n-Peff\n"},{"id":"104896","messageId":"7v63jbf2v0.fsf@gitster.siamese.dyndns.org","threadId":"17805","inReplyTo":"20090216033634.GA12461@coredump.intra.peff.net","subject":"Re: \"add -p\" + filenames with UTF-8 multibyte characters = \"No changes\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-16T04:00:03Z","receivedAt":"2009-02-16T04:00:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But what is the right encoding to specify? We can guess that it is\n> whatever the commit message is in (defaulting to utf-8). It is by no\n> means correct, but it would probably work pretty well in practice.\n>\n> On the other hand, we already have the same problem for encoded file\n> _contents_. So maybe it is not a big problem in practice.\n\nI did not spell the specifics out because this change won't happen in any\nnear future anyway, but my thinking was to give a way for \"add -p\" to\neither (1) internally run without quotepath regardless of the user's\nsettings or (2) unquote the paths correctly when it learns the set of\npaths affected by the change.\n\nI think the right approach is (2), because you need to unquote pathnames\nwith some byte values that even with core.quotepath=false will not pass\nunquoted *anyway*.\n\nI also happen to think that it may be a good idea to ignore core.quotepath\nsettings in format-patch, but that is a separate topic.\n"},{"id":"104905","messageId":"87ljs7ynf0.fsf@iki.fi","threadId":"17805","inReplyTo":"20090216033634.GA12461@coredump.intra.peff.net","subject":"Re: \"add -p\" + filenames with UTF-8 multibyte characters = \"No changes\"","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2009-02-16T05:13:23Z","receivedAt":"2009-02-16T05:13:23Z","isPatch":false,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"On 2009-02-15 22:36 (-0500), Jeff King wrote:\n\n> I am not opposed to setting this as a default, but I think there may\n> be some encoding issues to be dealt with. At the very least,\n> format-patch generates messages without a content-type header. E.g.,:\n\n> But what is the right encoding to specify? We can guess that it is\n> whatever the commit message is in (defaulting to utf-8). It is by no\n> means correct, but it would probably work pretty well in practice.\n\nI have a small script which adds/rewrites MIME headers. It defaults to\nUTF-8/8bit:\n\n    #!/bin/sh\n\n    charset=\"${1:-UTF-8}\"\n    encoding=\"${2:-8bit}\"\n\n    formail -I \"MIME-Version: 1.0\" \\\n            -I \"Content-Type: text/plain; charset=$charset\" \\\n            -I \"Content-Transfer-Encoding: $encoding\" \\\n            -s\n\nI have used the script this way:\n\n    git format-patch --stdout [...] | add-mime-headers | \\\n        formail -s sh -c 'cat >$FILENO.patch'\n\nIt may be difficult and unreliable to try to detect the encoding of file\ncontent so maybe \"git format-patch\" could be taught an option like\n\"--charset=<charset>\". (And perhaps also a configuration variable.) That\nwould just write MIME headers (charset) as the user wishes.\n"},{"id":"105081","messageId":"7v3aeda6lw.fsf_-_@gitster.siamese.dyndns.org","threadId":"17805","inReplyTo":"7v63jbf2v0.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-add -i/-p: learn to unwrap C-quoted paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T07:02:35Z","receivedAt":"2009-02-17T07:02:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The underlying plumbing commands are not run with -z option, so the paths\nreturned from them need to be unquoted as needed.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Junio C Hamano <gitster@pobox.com> writes:\n\n > I did not spell the specifics out because this change won't happen in any\n > near future anyway, but my thinking was to give a way for \"add -p\" to\n > either (1) internally run without quotepath regardless of the user's\n > settings or (2) unquote the paths correctly when it learns the set of\n > paths affected by the change.\n >\n > I think the right approach is (2), because you need to unquote pathnames\n > with some byte values that even with core.quotepath=false will not pass\n > unquoted *anyway*.\n\n I deliberately avoided using :utf8 because pathnames to git are always\n binary text, not necessarily utf8, and for the same reason I dropped the\n \"shortcut by first character\", as the pathnames are stored as raw\n sequence of bytes and using the first byte is obviously wrong.  It would\n be very much welcomed if somebody feels inclined to test and polish it.\n\n This hopefully makes the discussion on default value for core.quotepath\n independent of \"git-add -i/-p\".\n\n git-add--interactive.perl |   55 ++++++++++++++++++++++++++++++++++++++++++--\n 1 files changed, 52 insertions(+), 3 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 5f129a4..064d4c6 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -3,6 +3,8 @@\n use strict;\n use Git;\n \n+binmode(STDOUT, \":raw\");\n+\n my $repo = Git->repository();\n \n my $menu_use_color = $repo->get_colorbool('color.interactive');\n@@ -91,6 +93,47 @@ if (!defined $GIT_DIR) {\n }\n chomp($GIT_DIR);\n \n+my %cquote_map = (\n+ \"b\" => chr(8),\n+ \"t\" => chr(9),\n+ \"n\" => chr(10),\n+ \"v\" => chr(11),\n+ \"f\" => chr(12),\n+ \"r\" => chr(13),\n+ \"\\\\\" => \"\\\\\",\n+ \"\\042\" => \"\\042\",\n+);\n+\n+sub unquote_path {\n+\tlocal ($_) = @_;\n+\tmy ($retval, $remainder);\n+\tif (!/^\\042(.*)\\042$/) {\n+\t\treturn $_;\n+\t}\n+\t($_, $retval) = ($1, \"\");\n+\twhile (/^([^\\\\]*)\\\\(.*)$/) {\n+\t\t$remainder = $2;\n+\t\t$retval .= $1;\n+\t\tfor ($remainder) {\n+\t\t\tif (/^([0-3][0-7][0-7])(.*)$/) {\n+\t\t\t\t$retval .= chr(oct($1));\n+\t\t\t\t$_ = $2;\n+\t\t\t\tlast;\n+\t\t\t}\n+\t\t\tif (/^([\\\\\\042btnvfr])(.*)$/) {\n+\t\t\t\t$retval .= $cquote_map{$1};\n+\t\t\t\t$_ = $2;\n+\t\t\t\tlast;\n+\t\t\t}\n+\t\t\t# This is malformed -- just return it as-is for now.\n+\t\t\treturn $_[0];\n+\t\t}\n+\t\t$_ = $remainder;\n+\t}\n+\t$retval .= $_;\n+\treturn $retval;\n+}\n+\n sub refresh {\n \tmy $fh;\n \topen $fh, 'git update-index --refresh |'\n@@ -104,7 +147,7 @@ sub refresh {\n sub list_untracked {\n \tmap {\n \t\tchomp $_;\n-\t\t$_;\n+\t\tunquote_path($_);\n \t}\n \trun_cmd_pipe(qw(git ls-files --others --exclude-standard --), @ARGV);\n }\n@@ -141,7 +184,8 @@ sub list_modified {\n \n \tif (@ARGV) {\n \t\t@tracked = map {\n-\t\t\tchomp $_; $_;\n+\t\t\tchomp $_;\n+\t\t\tunquote_path($_);\n \t\t} run_cmd_pipe(qw(git ls-files --exclude-standard --), @ARGV);\n \t\treturn if (!@tracked);\n \t}\n@@ -153,6 +197,7 @@ sub list_modified {\n \t\tif (($add, $del, $file) =\n \t\t    /^([-\\d]+)\t([-\\d]+)\t(.*)/) {\n \t\t\tmy ($change, $bin);\n+\t\t\t$file = unquote_path($file);\n \t\t\tif ($add eq '-' && $del eq '-') {\n \t\t\t\t$change = 'binary';\n \t\t\t\t$bin = 1;\n@@ -168,6 +213,7 @@ sub list_modified {\n \t\t}\n \t\telsif (($adddel, $file) =\n \t\t       /^ (create|delete) mode [0-7]+ (.*)$/) {\n+\t\t\t$file = unquote_path($file);\n \t\t\t$data{$file}{INDEX_ADDDEL} = $adddel;\n \t\t}\n \t}\n@@ -175,6 +221,7 @@ sub list_modified {\n \tfor (run_cmd_pipe(qw(git diff-files --numstat --summary --), @tracked)) {\n \t\tif (($add, $del, $file) =\n \t\t    /^([-\\d]+)\t([-\\d]+)\t(.*)/) {\n+\t\t\t$file = unquote_path($file);\n \t\t\tif (!exists $data{$file}) {\n \t\t\t\t$data{$file} = +{\n \t\t\t\t\tINDEX => 'unchanged',\n@@ -196,6 +243,7 @@ sub list_modified {\n \t\t}\n \t\telsif (($adddel, $file) =\n \t\t       /^ (create|delete) mode [0-7]+ (.*)$/) {\n+\t\t\t$file = unquote_path($file);\n \t\t\t$data{$file}{FILE_ADDDEL} = $adddel;\n \t\t}\n \t}\n@@ -302,7 +350,8 @@ sub find_unique_prefixes {\n \t\t\t}\n \t\t\t%search = %{$search{$letter}};\n \t\t}\n-\t\tif ($soft_limit && $j + 1 > $soft_limit) {\n+\t\tif (ord($letters[0]) > 127 ||\n+\t\t    ($soft_limit && $j + 1 > $soft_limit)) {\n \t\t\t$prefix = undef;\n \t\t\t$remainder = $ret;\n \t\t}\n-- \n1.6.2.rc1.47.g30e1d8\n"},{"id":"105092","messageId":"878wo55vte.fsf@iki.fi","threadId":"17805","inReplyTo":"7v3aeda6lw.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-add -i/-p: learn to unwrap C-quoted paths","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2009-02-17T08:09:17Z","receivedAt":"2009-02-17T08:09:17Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"On 2009-02-16 23:02 (-0800), Junio C Hamano wrote:\n\n> The underlying plumbing commands are not run with -z option, so the paths\n> returned from them need to be unquoted as needed.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nAccording to my quick tests this seems to work. You may want to squash\nthis patch in:\n\n\ndiff --git i/Documentation/git-add.txt w/Documentation/git-add.txt\nindex 7c129cb..6c79a87 100644\n--- i/Documentation/git-add.txt\n+++ w/Documentation/git-add.txt\n@@ -263,13 +263,6 @@ diff::\n   This lets you review what will be committed (i.e. between\n   HEAD and index).\n \n-Bugs\n-----\n-The interactive mode does not work with files whose names contain\n-characters that need C-quoting.  `core.quotepath` configuration can be\n-used to work this limitation around to some degree, but backslash,\n-double-quote and control characters will still have problems.\n-\n SEE ALSO\n --------\n linkgit:git-status[1]\n"}]}