Re: [PATCH] Allow git-mergetool to handle paths with a leading space

Subsystems: the rest

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Allow git-mergetool to handle paths with a leading space

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:02

Rogan Dawes [off-list ref] writes:
quoted
quoted
I am working on a project which has the root directory constructed
with a leading space. i.e. ./ dir/. "read" skips the leading space
char, and ends up with an incorrect filename, which can then not be
found. Setting IFS=\n solves this problem.
Does the project have a file that has letter 'n' (en) in its name?
Have you tested your patch while having a conflict in that file?
Yes, it works correctly.
I am curious and puzzled...

        $ echo 'ann1' | (IFS=\n read i; echo "<$i>")
        <ann1>
        $ echo 'ann1' | (IFS=\n read i j; echo "<$i>")
	<a>
        $ echo 'n1' | (IFS=\n read i j; echo "<$i>")
	<>

Ok, "\n" is a funny way to say IFS does not matter as long as it
is set to a non whitespace letter.

It is VERY misleading as it looks as if the issue is fixed by
setting IFS to a single LF alone (excluding SP and HT from the
usual set), but that is not the patch is doing.  It is setting
it to a single 'n'.

I think you still will lose backslash by using read, but I guess
you would not care about that too much.

If you really cared, you would do something like this, but you
would also need similar surgery in merge_file function itself
that parses text form of ls-files output that tries to verify
and extract paths, which I did not bother to touch in this
demonstration patch.

 git-mergetool.sh |   33 ++++++++++++++++++++++-----------
 1 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 2f31fa2..b7c5098 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -388,17 +388,28 @@ fi
 
 
 if test $# -eq 0 ; then
-	files=`git ls-files -u | sed -e 's/^[^	]*	//' | sort -u`
-	if test -z "$files" ; then
-		echo "No files need merging"
-		exit 0
-	fi
-	echo Merging the files: $files
-	git ls-files -u | sed -e 's/^[^	]*	//' | sort -u | while read i
-	do
-		printf "\n"
-		merge_file "$i" < /dev/tty > /dev/tty
-	done
+	doit=$(perl -e '
+		$/ = "\0";
+		my (%seen, @file);
+		my $ls_files;
+		open($ls_files, "-|", qw(git ls-files -u -z));
+		while (<$ls_files>) {
+			chomp;
+			s/^[^	]*	//;
+			$seen{$_}++;
+		}
+		@file = sort keys %seen;
+		if (!@file) {
+			print "echo No files need merging\n";
+			print "exit 0\n";
+		}
+		for (@file) {
+			s|'\''|'\''\\'\'''\''|g;
+			print "echo\n";
+			print "merge_file '\''$_'\'' </dev/tty >/dev/tty\n";
+		}
+	')
+	eval "$doit"
 else
 	while test $# -gt 0; do
 		printf "\n"

[PATCH] Allow git-mergetool to handle paths with a leading space

From: Rogan Dawes <hidden>
Date: 2016-06-15 22:44:02

Signed-off-by: Rogan Dawes <redacted>
---
Junio C Hamano wrote:
Rogan Dawes [off-list ref] writes:
quoted
quoted
quoted
I am working on a project which has the root directory constructed
with a leading space. i.e. ./ dir/. "read" skips the leading space
char, and ends up with an incorrect filename, which can then not be
found. Setting IFS=\n solves this problem.
Does the project have a file that has letter 'n' (en) in its name?
Have you tested your patch while having a conflict in that file?
Yes, it works correctly.
I am curious and puzzled...

        $ echo 'ann1' | (IFS=\n read i; echo "<$i>")
        <ann1>
        $ echo 'ann1' | (IFS=\n read i j; echo "<$i>")
	<a>
        $ echo 'n1' | (IFS=\n read i j; echo "<$i>")
	<>

Ok, "\n" is a funny way to say IFS does not matter as long as it
is set to a non whitespace letter.

It is VERY misleading as it looks as if the issue is fixed by
setting IFS to a single LF alone (excluding SP and HT from the
usual set), but that is not the patch is doing.  It is setting
it to a single 'n'.
Yes, you are right. Maybe setting IFS to the empty string is better?

$ printf " ann 1\n ann 2\n" | while IFS="" read i j k ; do echo "<$i> 
<$j> <$k>"; done
< ann 1> <> <>
< ann 2> <> <>
$

Admittedly, there are still problems with my version, as you say, 
backslashes and newlines will not be handled correctly. My Perl-fu is 
weak, however, and my revised solution (IFS="") works for me :-)

Rogan

P.S. Quoting "$files" stops the spaces from being eaten in the preceding 
echo line.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 2f31fa2..facfbc8 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -393,8 +393,8 @@ if test $# -eq 0 ; then
                 echo "No files need merging"
                 exit 0
         fi
-       echo Merging the files: $files
-       git ls-files -u | sed -e 's/^[^ ]*      //' | sort -u | while read i
+       echo Merging the files: "$files"
+       git ls-files -u | sed -e 's/^[^ ]*      //' | sort -u | while 
IFS="" read i
         do
                 printf "\n"
                 merge_file "$i" < /dev/tty > /dev/tty
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help