Re: [PATCH/RFC] diff: Make numstat machine friendly also for renames

From: Junio C Hamano <junkio@cox.net>
Date: May 8, 2007, 01:58 UTC
Message-ID: <7vhcqo5b64.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <200705080345.26817.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

Show 14 quoted lines

>>> The numstat format for rename is now
>>>
>>>   added deleted TAB path for "src" TAB path for "dst" LF
>>>
>>> or if -z option is used
>>>
>>>   added deleted TAB path for "src" NUL NUL path for "dst" NUL
>> 
>> Why two NULs?
>
> That was the only way I could think of to separate pre-image name
> from posi-image name for renames. Note that file name might look like
> (part of) diffstat line, and there is no 'status' field in the
> numstat to mark rename (as there is in "git diff-tree --raw" output).

The --stat format is for human consumption, and --numstat (be it with -z or without) is for machines, so I am not opposed to a format change that gives information that is already computed but currently is hard to parse. If the format change breaks existing scripts, we might want to do --numstat-extended, though...

For example, I do not see a reason not to add "R98" in there. I.e.

	added deleted status TAB "src" (TAB "dst"){0,1} LF
	added deleted status NUL "src" (NUL "dst"){0,1} NUL

where the dst path is present only when status says it is a rename/copy, just like the --raw format.

> Did you mean --stat here?

No, I did mean --summary. But that was foolish of me. I forgot that it had the same { namepart => namepart } issue.

Show 14 quoted lines

>>> @@ -949,11 +955,19 @@ static void show_numstat(struct diffstat_t* data,
>>>  			printf("-\t-\t");
>>>  		else
>>>  			printf("%d\t%d\t", file->added, file->deleted);
>>> -		if (options->line_termination && !file->is_renamed &&
>>> +		if (options->line_termination &&
>>>  		    quote_c_style(file->name, NULL, NULL, 0))
>>>  			quote_c_style(file->name, NULL, stdout, 0);
>>>  		else
>>>  			fputs(file->name, stdout);
>>> +		if (file->is_renamed) {
>>> +			printf("%s", options->line_termination ? "\t" : "\0\0");
>
> It passes "make test".

But you already grepped to make sure this codepath is never exercised ;-).

What I was hoping you to notice was that printf("%s", "\0\0") thing. %s would not even notice that the const char[] literal is 2 bytes long.