Re: [PATCH/RFC] diff: Make numstat machine friendly also for renames
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:08
Jakub Narebski [off-list ref] writes:
Instead of saving human readable rename information in the 'name' field when diffstat info is generated, do it when writing --stat output. Change --numstat output to be machine friendly. This makes result of git-diff --numstat more suitable for machines also when renames are involved, by using format similar to the one for renames in the raw diff format, instead of the format more suited for humans. 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? There are already a handful in-tree users of --numstat, and also a few tests scripts. I think you would need to adjust them.
The goal of this change is to make it possible to generate HTML
diffstat against first parent for merge commits in gitweb. The current
notation for renames, which looks for example like below:
t/{t6030-bisect-run.sh => t6030-bisect-porcelain.sh}I do not have much objection against teaching --numstat to show the preimage pathnames. I do not disagree with "the goal" of showing "git diff --stat -M $commit^1 $commit" even for merge commit. But I do not see the connection between the two. Why aren't you parsing --summary? Have you actually _tested_ your patch?
quoted hunk
@@ -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");
I know you marked it as RFC; but it is impolite to request comments from other people on a patch that does not do what you intended to do, without marking "this is untested". It would waste people's time.