[PATCH 0/5] Move unquote_path() from git-add--interactive.perl to Git.pm

[PATCH 0/5] Move unquote_path() from git-add--interactive.perl to Git.pm

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

I'm using this in some scripts and it would be more convenient to have
it available from Git.pm rather than copying and pasting it each time
I need it. I think it should be useful to other people using Git.pm as
well. It is not uncommon to get a quoted path back from a command that
needs to be passed on the commandline of another command. While one
can use -z in many cases, that leaves the problem of having to quote
the path when printing it in error messages etc.

Phillip Wood (5):
  Git.pm Add unquote_path()
  Git::unquote_path() Handle '\a'
  Git::unquote_path() throw an exception on bad path
  Add tests for Git::unquote_path()
  git-add--interactive.perl: Use unquote_path() from Git.pm

 git-add--interactive.perl | 43 +------------------------------------
 perl/Git.pm               | 54 ++++++++++++++++++++++++++++++++++++++++++++++-
 t/t9700/test.pl           |  7 ++++++
 3 files changed, 61 insertions(+), 43 deletions(-)

-- 
2.13.0

[PATCH 1/5] Git.pm Add unquote_path()

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Add unquote_path() from git-add--interactive so it can be used by
other scripts. Note this is a straight copy, it does not handle
'\a'. That will be fixed in the next commit

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 perl/Git.pm | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index bfce1f795dfa5fea05f4f96637a1ae2333038735..8afde87fc8162271ba178e0fff3e921f070ac621 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -61,7 +61,8 @@ require Exporter;
                 remote_refs prompt
                 get_tz_offset get_record
                 credential credential_read credential_write
-                temp_acquire temp_is_locked temp_release temp_reset temp_path);
+                temp_acquire temp_is_locked temp_release temp_reset temp_path
+                unquote_path);
 
 
 =head1 DESCRIPTION
@@ -1451,6 +1452,56 @@ sub prefix_lines {
 	return $string;
 }
 
+=item unquote_path ( PATH )
+
+Unquote a quoted path containing c-escapes as returned by ls-files etc.
+when not using -z
+
+=cut
+
+{
+	my %unquote_map = (
+		"b" => chr(8),
+		"t" => chr(9),
+		"n" => chr(10),
+		"v" => chr(11),
+		"f" => chr(12),
+		"r" => chr(13),
+		"\\" => "\\",
+		"\"" => "\""
+	);
+
+	sub unquote_path {
+		local ($_) = @_;
+		my ($retval, $remainder);
+		if (!/^\042(.*)\042$/) {
+			return $_;
+		}
+		($_, $retval) = ($1, "");
+		while (/^([^\\]*)\\(.*)$/) {
+			$remainder = $2;
+			$retval .= $1;
+			for ($remainder) {
+				if (/^([0-3][0-7][0-7])(.*)$/) {
+					$retval .= chr(oct($1));
+					$_ = $2;
+					last;
+				}
+				if (/^([\\\042btnvfr])(.*)$/) {
+					$retval .= $unquote_map{$1};
+					$_ = $2;
+					last;
+				}
+				# This is malformed -- just return it as-is for now.
+				return $_[0];
+			}
+			$_ = $remainder;
+		}
+		$retval .= $_;
+		return $retval;
+	}
+}
+
 =item get_comment_line_char ( )
 
 Gets the core.commentchar configuration value.
-- 
2.13.0

Re: [PATCH 1/5] Git.pm Add unquote_path()

[Junio C Hamano]

Phillip Wood <phillip.wood@talktalk.net> writes:

> Subject: Re: [PATCH 1/5] Git.pm Add unquote_path()

Subject: [PATCH 1/5] Git.pm: add unquote_path()

But I think it is more customary to remove the implementation in the
other file and adjust the existing callers to call this new one in
this same commit.  And then in follow-up commits, improve the
implementation in Git.pm.

When that happens, the subject would become

Subject: [PATCH 1/?] add-i: move unquote_path() to Git.pm

and the first sentence in the body would be tweaked ("move
unquote_path() from ... to Git.pm so that ...").

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Add unquote_path() from git-add--interactive so it can be used by
> other scripts. Note this is a straight copy, it does not handle
> '\a'. That will be fixed in the next commit

s/next commit/&./

Good to notice and document the lack of '\a' which does get used by
quote.c::sq_lookup[] when showing chr(7).

> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  perl/Git.pm | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index bfce1f795dfa5fea05f4f96637a1ae2333038735..8afde87fc8162271ba178e0fff3e921f070ac621 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -61,7 +61,8 @@ require Exporter;
>                  remote_refs prompt
>                  get_tz_offset get_record
>                  credential credential_read credential_write
> -                temp_acquire temp_is_locked temp_release temp_reset temp_path);
> +                temp_acquire temp_is_locked temp_release temp_reset temp_path
> +                unquote_path);
>  
>  
>  =head1 DESCRIPTION
> @@ -1451,6 +1452,56 @@ sub prefix_lines {
>  	return $string;
>  }
>  
> +=item unquote_path ( PATH )
> +
> +Unquote a quoted path containing c-escapes as returned by ls-files etc.
> +when not using -z
> +
> +=cut
> +
> +{
> +	my %unquote_map = (

The original calls this cquote_map, partly because there may be
other kinds of quoting possible.  I do not see a reason not to
follow suit here.

> +		"b" => chr(8),
> +		"t" => chr(9),
> +		"n" => chr(10),
> +		"v" => chr(11),
> +		"f" => chr(12),
> +		"r" => chr(13),
> +		"\\" => "\\",
> +		"\"" => "\""

In an early part of "sub unquote_path" we see dq spelled as "\042"
and in the original cquote_map, this is also defined with "\042".  I
do not think you want to change the above to make them inconsistent.

> +	);
> +
> +	sub unquote_path {
> +		local ($_) = @_;
> +		my ($retval, $remainder);
> +		if (!/^\042(.*)\042$/) {
> +			return $_;
> +		}

Other than that, I can see this is just a straight-copy, which is
exactly what we want in an early step of a series like this.  Squash
in a change to remove the original from add-i and adjust the caller
to call this one (you may not have to do anything as it uses Git.pm
already, or perhaps "use Git qw(unquote_path)" or something?) and it
would be perfect ;-)

Thanks.

[PATCH 2/5] Git::unquote_path() Handle '\a'

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

The version copied from git-add--interactive did not handle quoted
paths containing '\a'.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 perl/Git.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 8afde87fc8162271ba178e0fff3e921f070ac621..889bf88cfcd34136e24e166fb3b72cced6debf9d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1461,6 +1461,7 @@ when not using -z
 
 {
 	my %unquote_map = (
+		"a" => chr(7),
 		"b" => chr(8),
 		"t" => chr(9),
 		"n" => chr(10),
@@ -1487,7 +1488,7 @@ when not using -z
 					$_ = $2;
 					last;
 				}
-				if (/^([\\\042btnvfr])(.*)$/) {
+				if (/^([\\\042abtnvfr])(.*)$/) {
 					$retval .= $unquote_map{$1};
 					$_ = $2;
 					last;
-- 
2.13.0

[PATCH 3/5] Git::unquote_path() throw an exception on bad path

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

This is what the other routines in Git.pm do if there's an error.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 perl/Git.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 889bf88cfcd34136e24e166fb3b72cced6debf9d..51cb3446088dd12e8fd93d47b95e29fab22a8466 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1493,8 +1493,8 @@ when not using -z
 					$_ = $2;
 					last;
 				}
-				# This is malformed -- just return it as-is for now.
-				return $_[0];
+				# This is malformed
+				throw Error::Simple("Invalid quoted path $_[0]");
 			}
 			$_ = $remainder;
 		}
-- 
2.13.0

Re: [PATCH 3/5] Git::unquote_path() throw an exception on bad path

[Junio C Hamano]

Phillip Wood <phillip.wood@talktalk.net> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> This is what the other routines in Git.pm do if there's an error.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  perl/Git.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index 889bf88cfcd34136e24e166fb3b72cced6debf9d..51cb3446088dd12e8fd93d47b95e29fab22a8466 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -1493,8 +1493,8 @@ when not using -z
>  					$_ = $2;
>  					last;
>  				}
> -				# This is malformed -- just return it as-is for now.
> -				return $_[0];
> +				# This is malformed
> +				throw Error::Simple("Invalid quoted path $_[0]");
>  			}

Output from "git grep Error::Simple" tells me that mostly we do not
upcase the first word of the error message.  Perhaps this should
follow the convention.

It is a different matter that use of Error::Simple has been
discouraged for quite some time (e.g. *1*) and we haven't been got
around doing that.  When the clean-up happens, this code will have
to be migrated to a different mechanism together with 30+ existing
uses of it; in the scope of this series, throwing of Error::Simple 
is consistent with the remainder of Git.pm and is a good thing.

[References]

*1* https://public-inbox.org/git/1330809281-25774-1-git-send-email-jnareb@gmail.com/

[PATCH 4/5] Add tests for Git::unquote_path()

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Check that unquote_path() handles spaces and escape sequences properly

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t9700/test.pl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 1b75c919651a8126e2a327f3d9645d4377823726..34cd01366f92164ffcd712cb86dac4eb1cffa3f5 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -133,6 +133,13 @@ close TEMPFILE3;
 unlink $tmpfile3;
 chdir($abs_repo_dir);
 
+# unquoting paths
+is(Git::unquote_path('abc'), 'abc', 'unquote unquoted path');
+is(Git::unquote_path('"abc def"'), 'abc def', 'unquote simple quoted path');
+is(Git::unquote_path('"abc\"\\\\ \a\b\t\n\v\f\r\001\040"'),
+		     "abc\"\\ \x07\x08\x09\x0a\x0b\x0c\x0d\x01 ",
+		     'unquote escape sequences');
+
 printf "1..%d\n", Test::More->builder->current_test;
 
 my $is_passing = eval { Test::More->is_passing };
-- 
2.13.0

[PATCH 5/5] git-add--interactive.perl: Use unquote_path() from Git.pm

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Not that we've copied unquote_path() to Git.pm use that copy instead
of our own.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl | 43 +------------------------------------------
 1 file changed, 1 insertion(+), 42 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 709a5f6ce6fbdb2da14084e94ae9df1db1c3d0a6..7ea3591edc056d4405b1f18800ffd2d0b351f93c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -3,7 +3,7 @@
 use 5.008;
 use strict;
 use warnings;
-use Git;
+use Git qw(unquote_path);
 use Git::I18N;
 
 binmode(STDOUT, ":raw");
@@ -175,47 +175,6 @@ if (!defined $GIT_DIR) {
 }
 chomp($GIT_DIR);
 
-my %cquote_map = (
- "b" => chr(8),
- "t" => chr(9),
- "n" => chr(10),
- "v" => chr(11),
- "f" => chr(12),
- "r" => chr(13),
- "\\" => "\\",
- "\042" => "\042",
-);
-
-sub unquote_path {
-	local ($_) = @_;
-	my ($retval, $remainder);
-	if (!/^\042(.*)\042$/) {
-		return $_;
-	}
-	($_, $retval) = ($1, "");
-	while (/^([^\\]*)\\(.*)$/) {
-		$remainder = $2;
-		$retval .= $1;
-		for ($remainder) {
-			if (/^([0-3][0-7][0-7])(.*)$/) {
-				$retval .= chr(oct($1));
-				$_ = $2;
-				last;
-			}
-			if (/^([\\\042btnvfr])(.*)$/) {
-				$retval .= $cquote_map{$1};
-				$_ = $2;
-				last;
-			}
-			# This is malformed -- just return it as-is for now.
-			return $_[0];
-		}
-		$_ = $remainder;
-	}
-	$retval .= $_;
-	return $retval;
-}
-
 sub refresh {
 	my $fh;
 	open $fh, 'git update-index --refresh |'
-- 
2.13.0

Re: [PATCH 0/5] Move unquote_path() from git-add--interactive.perl to Git.pm

[Junio C Hamano]

Thanks.  I had some comments, mostly on the structure of the series,
but overall it was a pleasant read that takes the code in the right
direction.

Re: [PATCH 0/5] Move unquote_path() from git-add--interactive.perl to Git.pm

[Jeff King]

On Thu, Jun 22, 2017 at 11:26:17AM +0100, Phillip Wood wrote:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> I'm using this in some scripts and it would be more convenient to have
> it available from Git.pm rather than copying and pasting it each time
> I need it. I think it should be useful to other people using Git.pm as
> well. It is not uncommon to get a quoted path back from a command that
> needs to be passed on the commandline of another command. While one
> can use -z in many cases, that leaves the problem of having to quote
> the path when printing it in error messages etc.

Grepping around the calls to unquote_path in add--interactive, I
definitely think many of them ought to be using "-z". But I don't think
that's a reason not to make unquote_path() more widely available. It
_is_ generally useful.

The changes look sane to me. My biggest question is how add--interactive
handles the exceptions thrown by the refactored function on error. Since
these paths are coming from Git, it should be something never comes up,
right? So failing hard is probably the best thing to do.

-Peff

Re: [PATCH 0/5] Move unquote_path() from git-add--interactive.perl to Git.pm

[Phillip Wood]

On 23/06/17 00:18, Jeff King wrote:
> On Thu, Jun 22, 2017 at 11:26:17AM +0100, Phillip Wood wrote:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> I'm using this in some scripts and it would be more convenient to have
>> it available from Git.pm rather than copying and pasting it each time
>> I need it. I think it should be useful to other people using Git.pm as
>> well. It is not uncommon to get a quoted path back from a command that
>> needs to be passed on the commandline of another command. While one
>> can use -z in many cases, that leaves the problem of having to quote
>> the path when printing it in error messages etc.
> 
> Grepping around the calls to unquote_path in add--interactive, I
> definitely think many of them ought to be using "-z". But I don't think
> that's a reason not to make unquote_path() more widely available. It
> _is_ generally useful.
>
> The changes look sane to me. My biggest question is how add--interactive
> handles the exceptions thrown by the refactored function on error. Since
> these paths are coming from Git, it should be something never comes up,
> right? So failing hard is probably the best thing to do.

Yes, my assumption was that the error shouldn't be triggered, and if it
was then it would be a sign of bigger problems.

Best Wishes

Phillip

[PATCH 0/4] Move unquote_path from git-add--interactive.perl to Git.pm

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Thanks for the review Junio, I've amended the patches as you suggested.

The first patch now removes unquote_path() from add -i as well as
adding it to Git.pm. I've fixed the naming issues (the version I sent
previously was copied from another script rather than directly from
add -i, I thought it was identical but must have changed it slightly
when I copied the original and then forgotten I'd made some changes)
I also expanded the documentation very slightly to mention parsing
diff -u as I think using -z isn't possible for that.

The error message in the third patch now starts with a lowercase letter.

Best Wishes

Phillip

Phillip Wood (4):
  add -i move unquote_path() to Git.pm
  Git::unquote_path() Handle '\a'
  Git::unquote_path() throw an exception on bad path
  Add tests for Git::unquote_path()

 git-add--interactive.perl | 43 +------------------------------------
 perl/Git.pm               | 54 ++++++++++++++++++++++++++++++++++++++++++++++-
 t/t9700/test.pl           |  7 ++++++
 3 files changed, 61 insertions(+), 43 deletions(-)

-- 
2.13.1

[PATCH 1/4] add -i move unquote_path() to Git.pm

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Move unquote_path() from git-add--interactive to Git.pm so it can be
used by other scripts. Note this is a straight copy, it does not
handle '\a'. That will be fixed in the next commit.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 git-add--interactive.perl | 43 +-------------------------------------
 perl/Git.pm               | 53 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 43 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 709a5f6ce6fbdb2da14084e94ae9df1db1c3d0a6..7ea3591edc056d4405b1f18800ffd2d0b351f93c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -3,7 +3,7 @@
 use 5.008;
 use strict;
 use warnings;
-use Git;
+use Git qw(unquote_path);
 use Git::I18N;
 
 binmode(STDOUT, ":raw");
@@ -175,47 +175,6 @@ if (!defined $GIT_DIR) {
 }
 chomp($GIT_DIR);
 
-my %cquote_map = (
- "b" => chr(8),
- "t" => chr(9),
- "n" => chr(10),
- "v" => chr(11),
- "f" => chr(12),
- "r" => chr(13),
- "\\" => "\\",
- "\042" => "\042",
-);
-
-sub unquote_path {
-	local ($_) = @_;
-	my ($retval, $remainder);
-	if (!/^\042(.*)\042$/) {
-		return $_;
-	}
-	($_, $retval) = ($1, "");
-	while (/^([^\\]*)\\(.*)$/) {
-		$remainder = $2;
-		$retval .= $1;
-		for ($remainder) {
-			if (/^([0-3][0-7][0-7])(.*)$/) {
-				$retval .= chr(oct($1));
-				$_ = $2;
-				last;
-			}
-			if (/^([\\\042btnvfr])(.*)$/) {
-				$retval .= $cquote_map{$1};
-				$_ = $2;
-				last;
-			}
-			# This is malformed -- just return it as-is for now.
-			return $_[0];
-		}
-		$_ = $remainder;
-	}
-	$retval .= $_;
-	return $retval;
-}
-
 sub refresh {
 	my $fh;
 	open $fh, 'git update-index --refresh |'
diff --git a/perl/Git.pm b/perl/Git.pm
index bfce1f795dfa5fea05f4f96637a1ae2333038735..35188842ef82c67f83f6d72fd37e38edb895328b 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -61,7 +61,8 @@ require Exporter;
                 remote_refs prompt
                 get_tz_offset get_record
                 credential credential_read credential_write
-                temp_acquire temp_is_locked temp_release temp_reset temp_path);
+                temp_acquire temp_is_locked temp_release temp_reset temp_path
+                unquote_path);
 
 
 =head1 DESCRIPTION
@@ -1451,6 +1452,56 @@ sub prefix_lines {
 	return $string;
 }
 
+=item unquote_path ( PATH )
+
+Unquote a quoted path containing c-escapes as returned by ls-files etc.
+when not using -z or when parsing the output of diff -u.
+
+=cut
+
+{
+	my %cquote_map = (
+		"b" => chr(8),
+		"t" => chr(9),
+		"n" => chr(10),
+		"v" => chr(11),
+		"f" => chr(12),
+		"r" => chr(13),
+		"\\" => "\\",
+		"\"" => "\042"
+	);
+
+	sub unquote_path {
+		local ($_) = @_;
+		my ($retval, $remainder);
+		if (!/^\042(.*)\042$/) {
+			return $_;
+		}
+		($_, $retval) = ($1, "");
+		while (/^([^\\]*)\\(.*)$/) {
+			$remainder = $2;
+			$retval .= $1;
+			for ($remainder) {
+				if (/^([0-3][0-7][0-7])(.*)$/) {
+					$retval .= chr(oct($1));
+					$_ = $2;
+					last;
+				}
+				if (/^([\\\042btnvfr])(.*)$/) {
+					$retval .= $cquote_map{$1};
+					$_ = $2;
+					last;
+				}
+				# This is malformed -- just return it as-is for now.
+				return $_[0];
+			}
+			$_ = $remainder;
+		}
+		$retval .= $_;
+		return $retval;
+	}
+}
+
 =item get_comment_line_char ( )
 
 Gets the core.commentchar configuration value.
-- 
2.13.1

[PATCH 2/4] Git::unquote_path() Handle '\a'

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

The version copied from git-add--interactive did not handle quoted
paths containing '\a'.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 perl/Git.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 35188842ef82c67f83f6d72fd37e38edb895328b..f075b500c510d183074c0358fb24fefc72248125 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1461,6 +1461,7 @@ when not using -z or when parsing the output of diff -u.
 
 {
 	my %cquote_map = (
+		"a" => chr(7),
 		"b" => chr(8),
 		"t" => chr(9),
 		"n" => chr(10),
@@ -1487,7 +1488,7 @@ when not using -z or when parsing the output of diff -u.
 					$_ = $2;
 					last;
 				}
-				if (/^([\\\042btnvfr])(.*)$/) {
+				if (/^([\\\042abtnvfr])(.*)$/) {
 					$retval .= $cquote_map{$1};
 					$_ = $2;
 					last;
-- 
2.13.1

[PATCH 3/4] Git::unquote_path() throw an exception on bad path

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

This is what the other routines in Git.pm do if there's an error.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 perl/Git.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index f075b500c510d183074c0358fb24fefc72248125..baf80d1ab038590c85bc5971483517cba606881f 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1493,8 +1493,8 @@ when not using -z or when parsing the output of diff -u.
 					$_ = $2;
 					last;
 				}
-				# This is malformed -- just return it as-is for now.
-				return $_[0];
+				# This is malformed
+				throw Error::Simple("invalid quoted path $_[0]");
 			}
 			$_ = $remainder;
 		}
-- 
2.13.1

[PATCH 4/4] Add tests for Git::unquote_path()

[Phillip Wood]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Check that unquote_path() handles spaces and escape sequences properly

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t9700/test.pl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 1b75c919651a8126e2a327f3d9645d4377823726..34cd01366f92164ffcd712cb86dac4eb1cffa3f5 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -133,6 +133,13 @@ close TEMPFILE3;
 unlink $tmpfile3;
 chdir($abs_repo_dir);
 
+# unquoting paths
+is(Git::unquote_path('abc'), 'abc', 'unquote unquoted path');
+is(Git::unquote_path('"abc def"'), 'abc def', 'unquote simple quoted path');
+is(Git::unquote_path('"abc\"\\\\ \a\b\t\n\v\f\r\001\040"'),
+		     "abc\"\\ \x07\x08\x09\x0a\x0b\x0c\x0d\x01 ",
+		     'unquote escape sequences');
+
 printf "1..%d\n", Test::More->builder->current_test;
 
 my $is_passing = eval { Test::More->is_passing };
-- 
2.13.1

Re: [PATCH 0/4] Move unquote_path from git-add--interactive.perl to Git.pm

[Junio C Hamano]

Looks good.  Will queue.

Thanks.