On 05/01/2012 08:27 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> writes:
>
>> Binary files chmoded without content change were reported as if they
>> were rewritten. At the same time, text files in the same situation
>> were reported as "unchanged". Let's treat binary files like text files
>> here, and simply say that they are unchanged.
>>
>> For text files, we knew that they were unchanged if the numbers of
>> lines added and deleted were both 0. For binary files this metric does
>> not make sense and is not calculated, so a new way of conveying this
>> information is needed. A new flag is_unchanged is added in struct
>> diffstat_t that is set if the contents of both files are identical.
>> For consistency, this new flag is used both for text files and binary
>> files.
>>
>> Output of --shortstat is modified in the same way.
>>
>> Reported-by: Martin Mareš <mj@xxxxxx>
>> Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx>
>> ---
>> diff.c | 28 +++++++++++++++++-----------
>> t/t4006-diff-mode.sh | 8 +-------
>> 2 files changed, 18 insertions(+), 18 deletions(-)
>>
>> diff --git a/diff.c b/diff.c
>> index 7da16c9..6eb2946 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -1299,6 +1299,7 @@ struct diffstat_t {
>> unsigned is_unmerged:1;
>> unsigned is_binary:1;
>> unsigned is_renamed:1;
>> + unsigned is_unchanged:1;
>
> The name is somewhat misleading, as a filepair that consists of two blobs
> with the same contents with different mode bits is still "changed", and
> you are trying to say that they have the same contents.
>
>> @@ -1471,7 +1472,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
>> struct diffstat_file *file = data->files[i];
>> uintmax_t change = file->added + file->deleted;
>> if (!data->files[i]->is_renamed &&
>> - (change == 0)) {
>> + data->files[i]->is_unchanged) {
>
> I am not sure if all these hunks are needed. If you are going to show
> only " Bin\n" for a filepair with the same binary contents, perhaps it is
> simpler to set added/deleted fields of such a filepair to 0? Then most of
> the hunks in this patch can disappear, no?
>
>> @@ -2379,6 +2383,8 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
>> return;
>> }
>>
>> + data->is_unchanged = hashcmp(one->sha1, two->sha1) == 0;
>
> Please write it as "!hashcmp(a, b)", not "hashcmp(a, b) == 0".
>
> In any case, how about doing it like this instead?
Yeah, this is much nicer.
On top of this, 4/4 becomes:
- else {
+ else if (hashcmp(one->sha1, two->sha1)) {
and the time improvement is the same (0.8 vs 2.0 s).
Do you want me to resend with your replacement patch?
Zbyszek
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html