Merge branch 'lt/diff-stat-show-0-lines' into maint
"git diff --stat" miscounted the total number of changed lines when binary files were involved and hidden beyond --stat-count. It also miscounted the total number of changed files when there were unmerged paths. * lt/diff-stat-show-0-lines: t4049: refocus tests diff --shortstat: do not count "unmerged" entries diff --stat: do not count "unmerged" entries diff --stat: move the "total count" logic to the last loop diff --stat: use "file" temporary variable to refer to data->files[i] diff --stat: status of unmodified pair in diff-q is not zero test: add failing tests for "diff --stat" to t4049 Fix "git diff --stat" for interesting - but empty - file changesmaint
						commit
						6a402843c2
					
				
							
								
								
									
										69
									
								
								diff.c
								
								
								
								
							
							
						
						
									
										69
									
								
								diff.c
								
								
								
								
							|  | @ -1300,6 +1300,7 @@ struct diffstat_t { | ||||||
| 		unsigned is_unmerged:1; | 		unsigned is_unmerged:1; | ||||||
| 		unsigned is_binary:1; | 		unsigned is_binary:1; | ||||||
| 		unsigned is_renamed:1; | 		unsigned is_renamed:1; | ||||||
|  | 		unsigned is_interesting:1; | ||||||
| 		uintmax_t added, deleted; | 		uintmax_t added, deleted; | ||||||
| 	} **files; | 	} **files; | ||||||
| }; | }; | ||||||
|  | @ -1469,8 +1470,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) | ||||||
| 	for (i = 0; (i < count) && (i < data->nr); i++) { | 	for (i = 0; (i < count) && (i < data->nr); i++) { | ||||||
| 		struct diffstat_file *file = data->files[i]; | 		struct diffstat_file *file = data->files[i]; | ||||||
| 		uintmax_t change = file->added + file->deleted; | 		uintmax_t change = file->added + file->deleted; | ||||||
| 		if (!data->files[i]->is_renamed && |  | ||||||
| 			 (change == 0)) { | 		if (!file->is_interesting && (change == 0)) { | ||||||
| 			count++; /* not shown == room for one more */ | 			count++; /* not shown == room for one more */ | ||||||
| 			continue; | 			continue; | ||||||
| 		} | 		} | ||||||
|  | @ -1497,7 +1498,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) | ||||||
| 		if (max_change < change) | 		if (max_change < change) | ||||||
| 			max_change = change; | 			max_change = change; | ||||||
| 	} | 	} | ||||||
| 	count = i; /* min(count, data->nr) */ | 	count = i; /* where we can stop scanning in data->files[] */ | ||||||
|  |  | ||||||
| 	/* | 	/* | ||||||
| 	 * We have width = stat_width or term_columns() columns total. | 	 * We have width = stat_width or term_columns() columns total. | ||||||
|  | @ -1585,16 +1586,15 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) | ||||||
| 	 */ | 	 */ | ||||||
| 	for (i = 0; i < count; i++) { | 	for (i = 0; i < count; i++) { | ||||||
| 		const char *prefix = ""; | 		const char *prefix = ""; | ||||||
| 		char *name = data->files[i]->print_name; | 		struct diffstat_file *file = data->files[i]; | ||||||
| 		uintmax_t added = data->files[i]->added; | 		char *name = file->print_name; | ||||||
| 		uintmax_t deleted = data->files[i]->deleted; | 		uintmax_t added = file->added; | ||||||
|  | 		uintmax_t deleted = file->deleted; | ||||||
| 		int name_len; | 		int name_len; | ||||||
|  |  | ||||||
| 		if (!data->files[i]->is_renamed && | 		if (!file->is_interesting && (added + deleted == 0)) | ||||||
| 			 (added + deleted == 0)) { |  | ||||||
| 			total_files--; |  | ||||||
| 			continue; | 			continue; | ||||||
| 		} |  | ||||||
| 		/* | 		/* | ||||||
| 		 * "scale" the filename | 		 * "scale" the filename | ||||||
| 		 */ | 		 */ | ||||||
|  | @ -1610,7 +1610,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) | ||||||
| 				name = slash; | 				name = slash; | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
| 		if (data->files[i]->is_binary) { | 		if (file->is_binary) { | ||||||
| 			fprintf(options->file, "%s", line_prefix); | 			fprintf(options->file, "%s", line_prefix); | ||||||
| 			show_name(options->file, prefix, name, len); | 			show_name(options->file, prefix, name, len); | ||||||
| 			fprintf(options->file, " %*s", number_width, "Bin"); | 			fprintf(options->file, " %*s", number_width, "Bin"); | ||||||
|  | @ -1627,7 +1627,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) | ||||||
| 			fprintf(options->file, "\n"); | 			fprintf(options->file, "\n"); | ||||||
| 			continue; | 			continue; | ||||||
| 		} | 		} | ||||||
| 		else if (data->files[i]->is_unmerged) { | 		else if (file->is_unmerged) { | ||||||
| 			fprintf(options->file, "%s", line_prefix); | 			fprintf(options->file, "%s", line_prefix); | ||||||
| 			show_name(options->file, prefix, name, len); | 			show_name(options->file, prefix, name, len); | ||||||
| 			fprintf(options->file, " Unmerged\n"); | 			fprintf(options->file, " Unmerged\n"); | ||||||
|  | @ -1639,8 +1639,6 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) | ||||||
| 		 */ | 		 */ | ||||||
| 		add = added; | 		add = added; | ||||||
| 		del = deleted; | 		del = deleted; | ||||||
| 		adds += add; |  | ||||||
| 		dels += del; |  | ||||||
|  |  | ||||||
| 		if (graph_width <= max_change) { | 		if (graph_width <= max_change) { | ||||||
| 			int total = add + del; | 			int total = add + del; | ||||||
|  | @ -1666,16 +1664,24 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) | ||||||
| 		show_graph(options->file, '-', del, del_c, reset); | 		show_graph(options->file, '-', del, del_c, reset); | ||||||
| 		fprintf(options->file, "\n"); | 		fprintf(options->file, "\n"); | ||||||
| 	} | 	} | ||||||
| 	for (i = count; i < data->nr; i++) { |  | ||||||
| 		uintmax_t added = data->files[i]->added; | 	for (i = 0; i < data->nr; i++) { | ||||||
| 		uintmax_t deleted = data->files[i]->deleted; | 		struct diffstat_file *file = data->files[i]; | ||||||
| 		if (!data->files[i]->is_renamed && | 		uintmax_t added = file->added; | ||||||
| 			 (added + deleted == 0)) { | 		uintmax_t deleted = file->deleted; | ||||||
|  |  | ||||||
|  | 		if (file->is_unmerged || | ||||||
|  | 		    (!file->is_interesting && (added + deleted == 0))) { | ||||||
| 			total_files--; | 			total_files--; | ||||||
| 			continue; | 			continue; | ||||||
| 		} | 		} | ||||||
| 		adds += added; |  | ||||||
| 		dels += deleted; | 		if (!file->is_binary) { | ||||||
|  | 			adds += added; | ||||||
|  | 			dels += deleted; | ||||||
|  | 		} | ||||||
|  | 		if (i < count) | ||||||
|  | 			continue; | ||||||
| 		if (!extra_shown) | 		if (!extra_shown) | ||||||
| 			fprintf(options->file, "%s ...\n", line_prefix); | 			fprintf(options->file, "%s ...\n", line_prefix); | ||||||
| 		extra_shown = 1; | 		extra_shown = 1; | ||||||
|  | @ -1695,9 +1701,8 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option | ||||||
| 		int added = data->files[i]->added; | 		int added = data->files[i]->added; | ||||||
| 		int deleted= data->files[i]->deleted; | 		int deleted= data->files[i]->deleted; | ||||||
|  |  | ||||||
| 		if (data->files[i]->is_unmerged) | 		if (data->files[i]->is_unmerged || | ||||||
| 			continue; | 		    (!data->files[i]->is_interesting && (added + deleted == 0))) { | ||||||
| 		if (!data->files[i]->is_renamed && (added + deleted == 0)) { |  | ||||||
| 			total_files--; | 			total_files--; | ||||||
| 		} else if (!data->files[i]->is_binary) { /* don't count bytes */ | 		} else if (!data->files[i]->is_binary) { /* don't count bytes */ | ||||||
| 			adds += added; | 			adds += added; | ||||||
|  | @ -2397,13 +2402,20 @@ static void builtin_diffstat(const char *name_a, const char *name_b, | ||||||
| 			     struct diff_filespec *two, | 			     struct diff_filespec *two, | ||||||
| 			     struct diffstat_t *diffstat, | 			     struct diffstat_t *diffstat, | ||||||
| 			     struct diff_options *o, | 			     struct diff_options *o, | ||||||
| 			     int complete_rewrite) | 			     struct diff_filepair *p) | ||||||
| { | { | ||||||
| 	mmfile_t mf1, mf2; | 	mmfile_t mf1, mf2; | ||||||
| 	struct diffstat_file *data; | 	struct diffstat_file *data; | ||||||
| 	int same_contents; | 	int same_contents; | ||||||
|  | 	int complete_rewrite = 0; | ||||||
|  |  | ||||||
|  | 	if (!DIFF_PAIR_UNMERGED(p)) { | ||||||
|  | 		if (p->status == DIFF_STATUS_MODIFIED && p->score) | ||||||
|  | 			complete_rewrite = 1; | ||||||
|  | 	} | ||||||
|  |  | ||||||
| 	data = diffstat_add(diffstat, name_a, name_b); | 	data = diffstat_add(diffstat, name_a, name_b); | ||||||
|  | 	data->is_interesting = p->status != DIFF_STATUS_UNKNOWN; | ||||||
|  |  | ||||||
| 	if (!one || !two) { | 	if (!one || !two) { | ||||||
| 		data->is_unmerged = 1; | 		data->is_unmerged = 1; | ||||||
|  | @ -3114,11 +3126,10 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o, | ||||||
| { | { | ||||||
| 	const char *name; | 	const char *name; | ||||||
| 	const char *other; | 	const char *other; | ||||||
| 	int complete_rewrite = 0; |  | ||||||
|  |  | ||||||
| 	if (DIFF_PAIR_UNMERGED(p)) { | 	if (DIFF_PAIR_UNMERGED(p)) { | ||||||
| 		/* unmerged */ | 		/* unmerged */ | ||||||
| 		builtin_diffstat(p->one->path, NULL, NULL, NULL, diffstat, o, 0); | 		builtin_diffstat(p->one->path, NULL, NULL, NULL, diffstat, o, p); | ||||||
| 		return; | 		return; | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -3131,9 +3142,7 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o, | ||||||
| 	diff_fill_sha1_info(p->one); | 	diff_fill_sha1_info(p->one); | ||||||
| 	diff_fill_sha1_info(p->two); | 	diff_fill_sha1_info(p->two); | ||||||
|  |  | ||||||
| 	if (p->status == DIFF_STATUS_MODIFIED && p->score) | 	builtin_diffstat(name, other, p->one, p->two, diffstat, o, p); | ||||||
| 		complete_rewrite = 1; |  | ||||||
| 	builtin_diffstat(name, other, p->one, p->two, diffstat, o, complete_rewrite); |  | ||||||
| } | } | ||||||
|  |  | ||||||
| static void run_checkdiff(struct diff_filepair *p, struct diff_options *o) | static void run_checkdiff(struct diff_filepair *p, struct diff_options *o) | ||||||
|  |  | ||||||
|  | @ -32,28 +32,28 @@ test_expect_success 'prepare binary file' ' | ||||||
| 	git commit -m binbin | 	git commit -m binbin | ||||||
| ' | ' | ||||||
|  |  | ||||||
| test_expect_success '--stat output after text chmod' ' | # test_expect_success '--stat output after text chmod' ' | ||||||
| 	test_chmod -x rezrov && | # 	test_chmod -x rezrov && | ||||||
| 	echo " 0 files changed" >expect && | # 	echo " 0 files changed" >expect && | ||||||
| 	git diff HEAD --stat >actual && | # 	git diff HEAD --stat >actual && | ||||||
| 	test_i18ncmp expect actual | #	test_i18ncmp expect actual | ||||||
| ' | # ' | ||||||
|  | # | ||||||
| test_expect_success '--shortstat output after text chmod' ' | # test_expect_success '--shortstat output after text chmod' ' | ||||||
| 	git diff HEAD --shortstat >actual && | # 	git diff HEAD --shortstat >actual && | ||||||
| 	test_i18ncmp expect actual | # 	test_i18ncmp expect actual | ||||||
| ' | # ' | ||||||
|  | # | ||||||
| test_expect_success '--stat output after binary chmod' ' | # test_expect_success '--stat output after binary chmod' ' | ||||||
| 	test_chmod +x binbin && | # 	test_chmod +x binbin && | ||||||
| 	echo " 0 files changed" >expect && | # 	echo " 0 files changed" >expect && | ||||||
| 	git diff HEAD --stat >actual && | # 	git diff HEAD --stat >actual && | ||||||
| 	test_i18ncmp expect actual | # 	test_i18ncmp expect actual | ||||||
| ' | # ' | ||||||
|  | # | ||||||
| test_expect_success '--shortstat output after binary chmod' ' | # test_expect_success '--shortstat output after binary chmod' ' | ||||||
| 	git diff HEAD --shortstat >actual && | # 	git diff HEAD --shortstat >actual && | ||||||
| 	test_i18ncmp expect actual | # 	test_i18ncmp expect actual | ||||||
| ' | # ' | ||||||
|  |  | ||||||
| test_done | test_done | ||||||
|  |  | ||||||
|  | @ -4,19 +4,62 @@ | ||||||
| test_description='diff --stat-count' | test_description='diff --stat-count' | ||||||
| . ./test-lib.sh | . ./test-lib.sh | ||||||
|  |  | ||||||
| test_expect_success setup ' | test_expect_success 'setup' ' | ||||||
| 	>a && | 	>a && | ||||||
| 	>b && | 	>b && | ||||||
| 	>c && | 	>c && | ||||||
| 	>d && | 	>d && | ||||||
| 	git add a b c d && | 	git add a b c d && | ||||||
| 	chmod +x c d && | 	git commit -m initial | ||||||
|  | ' | ||||||
|  |  | ||||||
|  | test_expect_success 'mode-only change show as a 0-line change' ' | ||||||
|  | 	git reset --hard && | ||||||
|  | 	test_chmod +x b d && | ||||||
|  | 	echo a >a && | ||||||
|  | 	echo c >c && | ||||||
|  | 	cat >expect <<-\EOF | ||||||
|  | 	 a | 1 + | ||||||
|  | 	 b | 0 | ||||||
|  | 	 ... | ||||||
|  | 	 4 files changed, 2 insertions(+) | ||||||
|  | 	EOF | ||||||
|  | 	git diff --stat --stat-count=2 HEAD >actual && | ||||||
|  | 	test_i18ncmp expect actual | ||||||
|  | ' | ||||||
|  |  | ||||||
|  | test_expect_success 'binary changes do not count in lines' ' | ||||||
|  | 	git reset --hard && | ||||||
|  | 	echo a >a && | ||||||
|  | 	echo c >c && | ||||||
|  | 	cat "$TEST_DIRECTORY"/test-binary-1.png >d && | ||||||
|  | 	cat >expect <<-\EOF | ||||||
|  | 	 a | 1 + | ||||||
|  | 	 c | 1 + | ||||||
|  | 	 ... | ||||||
|  | 	 3 files changed, 2 insertions(+) | ||||||
|  | 	EOF | ||||||
|  | 	git diff --stat --stat-count=2 >actual && | ||||||
|  | 	test_i18ncmp expect actual | ||||||
|  | ' | ||||||
|  |  | ||||||
|  | test_expect_success 'exclude unmerged entries from total file count' ' | ||||||
|  | 	git reset --hard && | ||||||
| 	echo a >a && | 	echo a >a && | ||||||
| 	echo b >b && | 	echo b >b && | ||||||
|  | 	git ls-files -s a >x && | ||||||
|  | 	git rm -f d && | ||||||
|  | 	for stage in 1 2 3 | ||||||
|  | 	do | ||||||
|  | 		sed -e "s/ 0	a/ $stage	d/" x | ||||||
|  | 	done | | ||||||
|  | 	git update-index --index-info && | ||||||
|  | 	echo d >d && | ||||||
| 	cat >expect <<-\EOF | 	cat >expect <<-\EOF | ||||||
| 	 a | 1 + | 	 a | 1 + | ||||||
| 	 b | 1 + | 	 b | 1 + | ||||||
| 	 2 files changed, 2 insertions(+) | 	 ... | ||||||
|  | 	 3 files changed, 3 insertions(+) | ||||||
| 	EOF | 	EOF | ||||||
| 	git diff --stat --stat-count=2 >actual && | 	git diff --stat --stat-count=2 >actual && | ||||||
| 	test_i18ncmp expect actual | 	test_i18ncmp expect actual | ||||||
|  |  | ||||||
|  | @ -85,7 +85,7 @@ test_expect_success 'NUL termination' ' | ||||||
|  |  | ||||||
| test_expect_success 'NUL separation with --stat' ' | test_expect_success 'NUL separation with --stat' ' | ||||||
| 	stat0_part=$(git diff --stat HEAD^ HEAD) && | 	stat0_part=$(git diff --stat HEAD^ HEAD) && | ||||||
| 	stat1_part=$(git diff --stat --root HEAD^) && | 	stat1_part=$(git diff-tree --no-commit-id --stat --root HEAD^) && | ||||||
| 	printf "add bar\n$stat0_part\n\0initial\n$stat1_part\n" >expected && | 	printf "add bar\n$stat0_part\n\0initial\n$stat1_part\n" >expected && | ||||||
| 	git log -z --stat --pretty="format:%s" >actual && | 	git log -z --stat --pretty="format:%s" >actual && | ||||||
| 	test_i18ncmp expected actual | 	test_i18ncmp expected actual | ||||||
|  | @ -93,7 +93,7 @@ test_expect_success 'NUL separation with --stat' ' | ||||||
|  |  | ||||||
| test_expect_failure 'NUL termination with --stat' ' | test_expect_failure 'NUL termination with --stat' ' | ||||||
| 	stat0_part=$(git diff --stat HEAD^ HEAD) && | 	stat0_part=$(git diff --stat HEAD^ HEAD) && | ||||||
| 	stat1_part=$(git diff --stat --root HEAD^) && | 	stat1_part=$(git diff-tree --no-commit-id --stat --root HEAD^) && | ||||||
| 	printf "add bar\n$stat0_part\n\0initial\n$stat1_part\n\0" >expected && | 	printf "add bar\n$stat0_part\n\0initial\n$stat1_part\n\0" >expected && | ||||||
| 	git log -z --stat --pretty="tformat:%s" >actual && | 	git log -z --stat --pretty="tformat:%s" >actual && | ||||||
| 	test_i18ncmp expected actual | 	test_i18ncmp expected actual | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Junio C Hamano
						Junio C Hamano