Breaks on files with spaces
dbfin opened this issue · 5 comments
icdiff 1.9.4 does not seem to work with file names containing spaces. There was a similar issue back in 2015, but I have a similar error now.
I also have additional observations compared to that issue.
It is very easy to reproduce and illustrate.
mkdir x && cd x && git init >/dev/null
echo 'a b 1' >'a b' && echo 'c 1' >c
git add . && git commit -m 'Initial commit.' >/dev/null
echo 'a b 2' >>'a b' && echo 'c 2' >>c
for t in diff icdiff; do
for h in '' 'HEAD -- '; do
cmd="git $t $h'a b' c"
echo -e '\nCOMMAND: '$cmd
eval $cmd
done
doneThe output:
COMMAND: git diff 'a b' c
diff --git a/a b b/a b
index cf5070d..df9599f 100644
--- a/a b
+++ b/a b
@@ -1 +1,2 @@
a b 1
+a b 2
diff --git a/c b/c
index b0403c1..387c380 100644
--- a/c
+++ b/c
@@ -1 +1,2 @@
c 1
+c 2
COMMAND: git diff HEAD -- 'a b' c
diff --git a/a b b/a b
index cf5070d..df9599f 100644
--- a/a b
+++ b/a b
@@ -1 +1,2 @@
a b 1
+a b 2
diff --git a/c b/c
index b0403c1..387c380 100644
--- a/c
+++ b/c
@@ -1 +1,2 @@
c 1
+c 2
COMMAND: git icdiff 'a b' c
fatal: ambiguous argument 'a': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
COMMAND: git icdiff HEAD -- 'a b' c
/tmp/Glmzh7_c c
c 1 c 1
c 2 Note the following:
git diffworks as expected both with and without 'HEAD --'.git icdiffwithout 'HEAD --' does not work at all when at least one filename contains a space.git icdiffwith 'HEAD --' does show differences but selectively only for filenames without spces.
Oh, and, of course, supplying several file names is not the issue here. git icdiff 'a b' would cause the error as above, and git icdiff HEAD -- 'a b' would produce the empty output (or maybe a single empty line) instead of output similar to file c above. I actually added file c just to illustrate what happens to the output of filenames without spaces when they are listed together with problematic ones in the two commands above, as both commands do produce output as expected if all supplied filenames are without spaces.
Does this work for you on icdiff master? I think it's possible #157 fixed this?
To confirm, this is only an issue with git icdiff, right, not when using icdiff plain?
I think I know now what the issue is. And yes, you do fix it #157, though that issue is not relevant, you just fix it at the same time.
As I said before, you already had this issue, and actually fixed it in #48. You fixed it yourself, but then others destroyed your fix :) -- you should really review more carefully which pull requests you accept.
In #115, the bug was reinvented in an irrelevant issue. Basically your correct version command "$@" was changed to "command arguments $@" instead of say "command arguments \"$@\"". Not only #115 destroyed your fix, but it also moved $@ to be the arguments of icdiff instead of git difftool. This second issue was fixed in #140 I think.
In #120, someone changed $@ to $*. You might look into that, what was the reason, because now it is changed back. :)
In #140, they pulled $* out of "" completely, essentially going all the way back to pre-#48 times.
Finally, again in an irrelevant issue #157, they changed $* to "$@", which both reinvented your 2015 fix for this issue, but also effectively cancelled changes made in #120 (what was the reason for that change anyway)?
I list all this so there will be references to this issue in those commits, and hopefully this problem won't appear anymore.
P.S. There is no single filename with spaces in tests/. It might be a good idea to add tests for this issue as well.
As a workaround for now I use
git difftool -y -x icdiff 'file name with spaces' instead of
git icdiff 'file name with spaces' It is a small inconvenience and does not take into account any icdiff parameters that could be set in .gitconfig, but I do not have any anyway.
This was fixed in #157 and will be in 1.9.5