Teach --dirstat not to completely ignore rearranged lines within a file
Currently, the --dirstat analysis ignores when lines within a file are rearranged, because the "damage" calculated by show_dirstat() is 0. However, if the object name has changed, we already know that there is some damage, and it is unintuitive to claim there is _no_ damage. Teach show_dirstat() to assign a minimum amount of damage (== 1) to entries for which the analysis otherwise yields zero damage, to still represent that these files are changed, instead of saying that there is no change. Also, skip --dirstat analysis when the object names are the same (e.g. for a pure file rename). Signed-off-by: Johan Herland <johan@herland.net> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>maint
							parent
							
								
									0133dab75d
								
							
						
					
					
						commit
						2ff3a80334
					
				|  | @ -74,8 +74,8 @@ endif::git-format-patch[] | ||||||
| 	counted for the parent directory, unless `--cumulative` is used. | 	counted for the parent directory, unless `--cumulative` is used. | ||||||
| + | + | ||||||
| Note that the `--dirstat` option computes the changes while ignoring | Note that the `--dirstat` option computes the changes while ignoring | ||||||
| pure code movements within a file.  In other words, rearranging lines | the amount of pure code movements within a file.  In other words, | ||||||
| in a file is not counted as a change. | rearranging lines in a file is not counted as much as other changes. | ||||||
|  |  | ||||||
| --dirstat-by-file[=<limit>]:: | --dirstat-by-file[=<limit>]:: | ||||||
| 	Same as `--dirstat`, but counts changed files instead of lines. | 	Same as `--dirstat`, but counts changed files instead of lines. | ||||||
|  |  | ||||||
							
								
								
									
										19
									
								
								diff.c
								
								
								
								
							
							
						
						
									
										19
									
								
								diff.c
								
								
								
								
							|  | @ -1548,6 +1548,16 @@ static void show_dirstat(struct diff_options *options) | ||||||
| 		else | 		else | ||||||
| 			content_changed = 1; | 			content_changed = 1; | ||||||
|  |  | ||||||
|  | 		if (!content_changed) { | ||||||
|  | 			/* | ||||||
|  | 			 * The SHA1 has not changed, so pre-/post-content is | ||||||
|  | 			 * identical. We can therefore skip looking at the | ||||||
|  | 			 * file contents altogether. | ||||||
|  | 			 */ | ||||||
|  | 			damage = 0; | ||||||
|  | 			goto found_damage; | ||||||
|  | 		} | ||||||
|  |  | ||||||
| 		if (DIFF_OPT_TST(options, DIRSTAT_BY_FILE)) { | 		if (DIFF_OPT_TST(options, DIRSTAT_BY_FILE)) { | ||||||
| 			/* | 			/* | ||||||
| 			 * In --dirstat-by-file mode, we don't really need to | 			 * In --dirstat-by-file mode, we don't really need to | ||||||
|  | @ -1556,7 +1566,7 @@ static void show_dirstat(struct diff_options *options) | ||||||
| 			 * add this file to the list of results | 			 * add this file to the list of results | ||||||
| 			 * (with each file contributing equal damage). | 			 * (with each file contributing equal damage). | ||||||
| 			 */ | 			 */ | ||||||
| 			damage = content_changed ? 1 : 0; | 			damage = 1; | ||||||
| 			goto found_damage; | 			goto found_damage; | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
|  | @ -1583,8 +1593,15 @@ static void show_dirstat(struct diff_options *options) | ||||||
| 		 * Original minus copied is the removed material, | 		 * Original minus copied is the removed material, | ||||||
| 		 * added is the new material.  They are both damages | 		 * added is the new material.  They are both damages | ||||||
| 		 * made to the preimage. | 		 * made to the preimage. | ||||||
|  | 		 * If the resulting damage is zero, we know that | ||||||
|  | 		 * diffcore_count_changes() considers the two entries to | ||||||
|  | 		 * be identical, but since content_changed is true, we | ||||||
|  | 		 * know that there must have been _some_ kind of change, | ||||||
|  | 		 * so we force all entries to have damage > 0. | ||||||
| 		 */ | 		 */ | ||||||
| 		damage = (p->one->size - copied) + added; | 		damage = (p->one->size - copied) + added; | ||||||
|  | 		if (!damage) | ||||||
|  | 			damage = 1; | ||||||
|  |  | ||||||
| found_damage: | found_damage: | ||||||
| 		ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc); | 		ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc); | ||||||
|  |  | ||||||
|  | @ -300,9 +300,7 @@ diff --no-index --name-status -- dir2 dir | ||||||
| diff --no-index dir dir3 | diff --no-index dir dir3 | ||||||
| diff master master^ side | diff master master^ side | ||||||
| diff --dirstat master~1 master~2 | diff --dirstat master~1 master~2 | ||||||
| # --dirstat doesn't notice changes that simply rearrange existing lines |  | ||||||
| diff --dirstat initial rearrange | diff --dirstat initial rearrange | ||||||
| # ...but --dirstat-by-file does notice changes that only rearrange lines |  | ||||||
| diff --dirstat-by-file initial rearrange | diff --dirstat-by-file initial rearrange | ||||||
| EOF | EOF | ||||||
|  |  | ||||||
|  |  | ||||||
|  | @ -1,2 +1,3 @@ | ||||||
| $ git diff --dirstat initial rearrange | $ git diff --dirstat initial rearrange | ||||||
|  |  100.0% dir/ | ||||||
| $ | $ | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Johan Herland
						Johan Herland