[PATCH] diffstat: fix parsing of filenames

[Gastón Kleiman]

On Tue, Feb 1, 2011 at 17:07, Matt Mackall <mpm at selenic.com> wrote:
> On Tue, 2011-02-01 at 15:39 -0300, Gastón Kleiman wrote:
>> # HG changeset patch
>> # User Gastón Kleiman <gaston.kleiman at gmail.com>
>> # Date 1296582352 10800
>> # Branch stable
>> # Node ID 0f59f053726484df622651a51c18309f700404b2
>> # Parent  a939f08fae9c4a51cc9bf4b3d9c512703856df84
>> diffstat: fix parsing of filenames
>>
>> This patch fixes the parsing of filenames which contain spaces.
>
> It'd be massively helpful if you told us what precisely you think was
> wrong, preferably with before and after examples. For all I can tell
> from your description, whatever you're fixing may actually have been
> that way intentionally.

I've included some tests in the patch, but you are right that the
description wasn't verbose enough.

The current stable version of Mercurial has the following behavior:

$ echo "foobar">"file with spaces"
$ hg add "file with spaces"
$ hg diff --stat
 spaces |  1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
$ hg diff --git --stat
 file with spaces |  1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

I think that in that case, the ouput of 'hg diff --stat' should show
the complete file name, just like 'hg diff --git --stat' does. At
first I thought that it could be intentional to be compatible with
broken tools, but after having looked at the code, I reached the
conclusion that it is a bug.

I originally fixed it by using the following line instead of a regex:

filename = line[line.rindex('-r'):].split(None, 2)[-1]

And then proceeded to commit the change with the message like this
one: "diffstat: Fix blah blah blah".

Later I tried to send you the patch using patchbomb, and it crashed.
The cause of the crash was patchbomb calling diffstatdata(lines), with
lines consisting of only the commit message. As my commit message
started with 'diff', hg tried to extract a filename from it and
crashed.

I don't know whether patchbomb's behaviour (passing the commit
messages to diffstatdata) is buggy or not, so I decided to test if the
string starts with "diff -r", and then to use a regex.

I hope that this description is complete enough, but just tell me if
you need something else =)

--
Gastón Kleiman