Nanako Shiraishi <nanako3@xxxxxxxxxxx> writes: > I need this extra script because setting 'nkf -w' for > textconv like this > > [diff "eucjp"] > textconv = nkf -w > > gives an error. > > % diff --git a/hello.txt b/hello.txt > index 696acd7..f07aa1a 100644 > error: cannot run nkf -w: No such file or directory > error: error running textconv command 'nkf -w' > fatal: unable to read files to diff > > Could you fix textconv so that it can be given parameters? The change to do so looks like this; it has a few side effects: - If somebody else were relying on the fact that 'nkf -w' names the entire command, it now will run 'nkf' command with '-w' as an argument, and it will break such a set-up. IOW, command that has an IFS white space in its path will now need to be quoted from the shell. You can see the fallout from this in the damage made to t/ hierarchy in the attached patch. - You can now use $HOME and other environment variables your shell expands when defining your textconv command. Overall I think it is a good direction to go, but we need to be careful about how we transition the existing repositories that use the old semantics. We might need to introduce diff.*.xtextconv or something. diff.c | 9 ++++----- t/t4030-diff-textconv.sh | 2 +- t/t4031-diff-rewrite-binary.sh | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/diff.c b/diff.c index 08bbd3e..64a1486 100644 --- a/diff.c +++ b/diff.c @@ -3760,15 +3760,14 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec, size_t *outsize) { struct diff_tempfile *temp; - const char *argv[3]; - const char **arg = argv; + const char *argv[4] = { "sh", "-c", NULL, NULL }; struct child_process child; struct strbuf buf = STRBUF_INIT; + struct strbuf cmd = STRBUF_INIT; temp = prepare_temp_file(spec->path, spec); - *arg++ = pgm; - *arg++ = temp->name; - *arg = NULL; + strbuf_addf(&cmd, "%s %s", pgm, temp->name); + argv[2] = strbuf_detach(&cmd, NULL); memset(&child, 0, sizeof(child)); child.argv = argv; diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index a3f0897..3468f77 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -48,7 +48,7 @@ test_expect_success 'file is considered binary by plumbing' ' test_expect_success 'setup textconv filters' ' echo file diff=foo >.gitattributes && - git config diff.foo.textconv "$PWD"/hexdump && + git config diff.foo.textconv \""$PWD"/hexdump\" && git config diff.fail.textconv false ' diff --git a/t/t4031-diff-rewrite-binary.sh b/t/t4031-diff-rewrite-binary.sh index a894c60..e6cb30e 100755 --- a/t/t4031-diff-rewrite-binary.sh +++ b/t/t4031-diff-rewrite-binary.sh @@ -54,7 +54,7 @@ chmod +x dump test_expect_success 'setup textconv' ' echo file diff=foo >.gitattributes && - git config diff.foo.textconv "$PWD"/dump + git config diff.foo.textconv \""$PWD"/dump\" ' test_expect_success 'rewrite diff respects textconv' ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html