`git status --porcelain` disagrees with documentation about quoting filenames with spaces

`git status --porcelain` disagrees with documentation about quoting filenames with spaces

[Kevin Ballard]

According to the manpage for git-status:

       The fields (including the ->) are separated from each other by a single
       space. If a filename contains whitespace or other nonprintable
       characters, that field will be quoted in the manner of a C string
       literal: surrounded by ASCII double quote (34) characters, and with
       interior special characters backslash-escaped.

This is no longer true for filenames with spaces in them. I did some digging and I believe this changed 5 years ago in 28fba290 (Do not quote SP). My inclination is to say that the documented behavior should be restored, at least for the case of a copy/rename, simply for the sake of removing ambiguity in weird cases like the following:

> ls
a  b
> git mv a 'a -> a with spaces'
> git status --porcelain
R  a -> a -> a with spaces

Given that --porcelain is supposed to be machine-readable, this last line is unexpected, as it can be parsed in two different ways. Restoring the behavior of quoting filenames with spaces, at least for copies/renames, would fix this problem.

Given that the removal of quoting for filenames with spaces was an intentional change, does anybody have any strong opinions about whether we should restore the quotes in this scenario? The alternative is to simply change the documentation, but the un-parsability of the --porcelain format has me worried.

Note: this was prompted by a user on the #git IRC channel who was trying to parse the --porcelain format, specifically for parsing renames, and was surprised by the output not conforming to the documentation. I pointed him at the -z flag, but I still believe the non-z case should be fixed.

-Kevin Ballard

Re: `git status --porcelain` disagrees with documentation about quoting filenames with spaces

[Junio C Hamano]

Kevin Ballard <kevin@sb.org> writes:

[jc: why do you send messages with toooooooooooo loooooong lines sometimes
and normal line lengths some other times...?]

> Given that the removal of quoting for filenames with spaces was an
> intentional change, does anybody have any strong opinions about whether
> we should restore the quotes in this scenario? The alternative is to
> simply change the documentation, but the un-parsability of the
> --porcelain format has me worried.

28fba29 (Do not quote SP., 2005-10-17) explicitly addressed a breakage
that quoted pathnames in contexts like this one:

    diff --git a/My Documents/hello.txt b/My Documents/hello.txt

I personally think people who add SP to their pathnames need to get their
head examined, and in that sense I do not strongly mind if the pathnames
in the above are quoted (that is why the original quotation before the
said commit quoted them), but apparently other people did mind.  I also
think people who have " -> " in their pathnames are even less sane beyond
salvation, and between the two insanity, I'd rather help less insane ones
by not quoting the above.

The best would probably be to special case SP (which is normally not to be
quoted) _only_ in the context of "something" -> "something".

Re: `git status --porcelain` disagrees with documentation about quoting filenames with spaces

[Kevin Ballard]

On Oct 28, 2010, at 11:44 AM, Junio C Hamano wrote:

> Kevin Ballard <kevin@sb.org> writes:
> 
> [jc: why do you send messages with toooooooooooo loooooong lines sometimes
> and normal line lengths some other times...?]

I use a GUI mail client to write email. Anything I copy&paste is hard-wrapped,
anything I write directly tends to not include hard linebreaks at all. Would it
be better if I hard-wrapped my lines?

>> Given that the removal of quoting for filenames with spaces was an
>> intentional change, does anybody have any strong opinions about whether
>> we should restore the quotes in this scenario? The alternative is to
>> simply change the documentation, but the un-parsability of the
>> --porcelain format has me worried.
> 
> 28fba29 (Do not quote SP., 2005-10-17) explicitly addressed a breakage
> that quoted pathnames in contexts like this one:
> 
>    diff --git a/My Documents/hello.txt b/My Documents/hello.txt
> 
> I personally think people who add SP to their pathnames need to get their
> head examined, and in that sense I do not strongly mind if the pathnames
> in the above are quoted (that is why the original quotation before the
> said commit quoted them), but apparently other people did mind.  I also
> think people who have " -> " in their pathnames are even less sane beyond
> salvation, and between the two insanity, I'd rather help less insane ones
> by not quoting the above.

I agree that SP in pathnames in source is insane, but it's perfectly common to
do when working with non-source files. For example, in the project I'm using
right now, I have a file named "Application 1.0.xcdatamodel" and another named
"Application 1.1.xcdatamodel". These are data files and their name on disk
matches the name given to them in the IDE. In this case, I think the SP is
perfectly justified. Granted, having " -> " in your pathname is also pretty
insane, but my motivation here is just ensuring that the --porcelain format
is parseable even if you are insane.

> The best would probably be to special case SP (which is normally not to be
> quoted) _only_ in the context of "something" -> "something".

That's what I was thinking. I'll look into doing just that.

-Kevin Ballard

Re: `git status --porcelain` disagrees with documentation about quoting filenames with spaces

[Junio C Hamano]

Kevin Ballard <kevin@sb.org> writes:

> On Oct 28, 2010, at 11:44 AM, Junio C Hamano wrote:
>
>> Kevin Ballard <kevin@sb.org> writes:
>> 
>> [jc: why do you send messages with toooooooooooo loooooong lines sometimes
>> and normal line lengths some other times...?]
>
> I use a GUI mail client to write email. Anything I copy&paste is hard-wrapped,
> anything I write directly tends to not include hard linebreaks at all. Would it
> be better if I hard-wrapped my lines?

It is not better vs worse but is acceptable vs unacceptable, as hard
wrapped messages have been the norm around here from day one.  As far as I
remember you only recently started sending messages with long lines, so I
suspected perhaps you changed your environment and are doing so without
realizing the pain you are causing to others.

> ... Granted, having " -> " in your pathname is also pretty
> insane, but my motivation here is just ensuring that the --porcelain format
> is parseable even if you are insane.

It is not just "also" but order of magnitude more insane ;-)

Just in case you misunderstood me, even though I think SP in path is
already insane, I am sympathetic enough to them to stand behind 28fba29
(Do not quote SP., 2005-10-17); they were the ones who complained when
they saw

    diff --git "a/My Documents/hello.txt" "b/My Documents/hello.txt"

and complained.  As 

    diff --git a/My Documents/hello.txt b/My Documents/hello.txt

is perfectly parsable (and no, don't worry about renames---we have extra
headers to keep them unambiguous), it is easier on their eyes if we did
not quote these paths that are "normal" to them ;-)

>> The best would probably be to special case SP (which is normally not to be
>> quoted) _only_ in the context of "something" -> "something".
>
> That's what I was thinking. I'll look into doing just that.

Yeah, if we wanted to be perfect, it would be better to do so without
causing unnecessary pain.

Re: `git status --porcelain` disagrees with documentation about quoting filenames with spaces

[Kevin Ballard]

On Oct 28, 2010, at 4:41 PM, Junio C Hamano wrote:

>>> Kevin Ballard <kevin@sb.org> writes:
>>> 
>>> [jc: why do you send messages with toooooooooooo loooooong lines sometimes
>>> and normal line lengths some other times...?]
>> 
>> I use a GUI mail client to write email. Anything I copy&paste is hard-wrapped,
>> anything I write directly tends to not include hard linebreaks at all. Would it
>> be better if I hard-wrapped my lines?
> 
> It is not better vs worse but is acceptable vs unacceptable, as hard
> wrapped messages have been the norm around here from day one.  As far as I
> remember you only recently started sending messages with long lines, so I
> suspected perhaps you changed your environment and are doing so without
> realizing the pain you are causing to others.

I apologize, I did not realize it caused a problem to others. I'm not used
to interacting with people using terminal mail clients (e.g. mutt). I didn't
give it much thought, but I just assumed the ML was hard-wrapping everybody's
messages (of course this is obviously wrong, as evidenced by my own messages).
I will bear this in mind for future emails.

>>> The best would probably be to special case SP (which is normally not to be
>>> quoted) _only_ in the context of "something" -> "something".
>> 
>> That's what I was thinking. I'll look into doing just that.
> 
> Yeah, if we wanted to be perfect, it would be better to do so without
> causing unnecessary pain.

Turns out it's fairly simple to do.

BTW, I'm trying an experiment here to see if I can just paste the patch into
Mail.app without it being mangled. I sent it to myself first, and Mail.app
is applying quoted-printable encoding to the patch, but it appears git-am
can still understand it. Please let me know if this isn't acceptable and I
will send it separately.

---8<---
Subject: status: Quote renamed/copied paths with spaces in short format

According to the documentation for git-status, in short output mode,
paths with spaces or unprintable characters are quoted. However
28fba29 (Do not quote SP., 2005-10-17) removed the behavior that quotes
paths that have spaces but not unprintable characters. Unfortunately this
makes the output of `git status --porcelain` non-parseable in certain
(rather unusual) edge cases. In the interest of removing ambiguity when
parsing the output of `git status --porcelain`, restore the behavior of
quoting paths with spaces but only for renamed/copied paths.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
wt-status.c |   10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index fc2438f..9624865 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -744,10 +744,20 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
		const char *one;
		if (d->head_path) {
			one = quote_path(d->head_path, -1, &onebuf, s->prefix);
+			if (*one != '"' && strchr(one, ' ') != NULL) {
+				putchar('"');
+				strbuf_addch(&onebuf, '"');
+				one = onebuf.buf;
+			}
			printf("%s -> ", one);
			strbuf_release(&onebuf);
		}
		one = quote_path(it->string, -1, &onebuf, s->prefix);
+		if (*one != '"' && strchr(one, ' ') != NULL) {
+			putchar('"');
+			strbuf_addch(&onebuf, '"');
+			one = onebuf.buf;
+		}
		printf("%s\n", one);
		strbuf_release(&onebuf);
	}
-- 
1.7.3.2.195.ge42d1.dirty

Re: `git status --porcelain` disagrees with documentation about quoting filenames with spaces

[Kevin Ballard]

If you don't mind I've brought this back onto the list.

On Oct 28, 2010, at 6:32 PM, Junio C Hamano wrote:

>> BTW, I'm trying an experiment here to see if I can just paste the patch into
>> Mail.app without it being mangled. I sent it to myself first, and Mail.app
>> is applying quoted-printable encoding to the patch, but it appears git-am
>> can still understand it. Please let me know if this isn't acceptable and I
>> will send it separately.
> 
> Almost but not quite; it appears that all the leading SP on the context
> lines and the diffstat are lost by somebody.

Well, it looks like I screwed up. I sent a test email to myself and it came
through fine, so I selected all, hit copy, and pasted that into the new
reply to the list. Unfortunately, copying from Mail.app's rich markup view
seems to have lost the spaces. If I copy from the Raw Source view it works
fine. I'll have to try again without making that mistake.

> The patch seems to unconditionally dq even when there is no rename
> (i.e. when d->head_path is NULL).
> 
> I think it _is_ intended (otherwise it becomes unwieldy to tell if you
> renamed "foo" to "bar" or if you touched "foo -> bar" without looking at
> the status letters) but the behaviour does not seem to match what the log
> message says it does.

Good point. I hadn't thought this through properly. Here's an updated patch
with a fixed description. And this time I'm not copying it from a test email ;)

---8<---
Subject: status: Quote paths with spaces in short format

According to the documentation for git-status, in short-format mode,
paths with spaces or unprintable characters are quoted. However
28fba29 (Do not quote SP., 2005-10-17) removed the behavior that quotes
paths that have spaces but not unprintable characters. Unfortunately this
makes the output of `git status --porcelain` non-parseable in certain
(rather unusual) edge cases. In the interest of removing ambiguity when
parsing the output of `git status --porcelain`, restore the behavior of
quoting paths with spaces in git-status's short-format mode.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
 wt-status.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index fc2438f..9624865 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -744,10 +744,20 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 		const char *one;
 		if (d->head_path) {
 			one = quote_path(d->head_path, -1, &onebuf, s->prefix);
+			if (*one != '"' && strchr(one, ' ') != NULL) {
+				putchar('"');
+				strbuf_addch(&onebuf, '"');
+				one = onebuf.buf;
+			}
 			printf("%s -> ", one);
 			strbuf_release(&onebuf);
 		}
 		one = quote_path(it->string, -1, &onebuf, s->prefix);
+		if (*one != '"' && strchr(one, ' ') != NULL) {
+			putchar('"');
+			strbuf_addch(&onebuf, '"');
+			one = onebuf.buf;
+		}
 		printf("%s\n", one);
 		strbuf_release(&onebuf);
 	}
-- 
1.7.3.2.195.ge42d1.dirty

[PATCH] status: Quote paths with spaces in short format

[Kevin Ballard]

According to the documentation for git-status, in short-format mode,
paths with spaces or unprintable characters are quoted. However
28fba29 (Do not quote SP., 2005-10-17) removed the behavior that quotes
paths that have spaces but not unprintable characters. Unfortunately this
makes the output of `git status --porcelain` non-parseable in certain
(rather unusual) edge cases. In the interest of removing ambiguity when
parsing the output of `git status --porcelain`, restore the behavior of
quoting paths with spaces in git-status's short-format mode.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---

This patch was originally attached via scissors to message id
<A2E979E4-899B-4295-A8CF-72EF8E585D3A@sb.org> but it appears to have
been overlooked.

 wt-status.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index fc2438f..9624865 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -744,10 +744,20 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 		const char *one;
 		if (d->head_path) {
 			one = quote_path(d->head_path, -1, &onebuf, s->prefix);
+			if (*one != '"' && strchr(one, ' ') != NULL) {
+				putchar('"');
+				strbuf_addch(&onebuf, '"');
+				one = onebuf.buf;
+			}
 			printf("%s -> ", one);
 			strbuf_release(&onebuf);
 		}
 		one = quote_path(it->string, -1, &onebuf, s->prefix);
+		if (*one != '"' && strchr(one, ' ') != NULL) {
+			putchar('"');
+			strbuf_addch(&onebuf, '"');
+			one = onebuf.buf;
+		}
 		printf("%s\n", one);
 		strbuf_release(&onebuf);
 	}
-- 
1.7.3.2.203.ge51db