Skip to content

Commit

Permalink
Teach --dirstat not to completely ignore rearranged lines within a file
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Johan Herland authored and Junio C Hamano committed Apr 11, 2011
1 parent 0133dab commit 2ff3a80
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 5 deletions.
4 changes: 2 additions & 2 deletions Documentation/diff-options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ endif::git-format-patch[]
counted for the parent directory, unless `--cumulative` is used.
+
Note that the `--dirstat` option computes the changes while ignoring
pure code movements within a file. In other words, rearranging lines
in a file is not counted as a change.
the amount of pure code movements within a file. In other words,
rearranging lines in a file is not counted as much as other changes.

--dirstat-by-file[=<limit>]::
Same as `--dirstat`, but counts changed files instead of lines.
Expand Down
19 changes: 18 additions & 1 deletion diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -1548,6 +1548,16 @@ static void show_dirstat(struct diff_options *options)
else
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)) {
/*
* In --dirstat-by-file mode, we don't really need to
Expand All @@ -1556,7 +1566,7 @@ static void show_dirstat(struct diff_options *options)
* add this file to the list of results
* (with each file contributing equal damage).
*/
damage = content_changed ? 1 : 0;
damage = 1;
goto found_damage;
}

Expand All @@ -1583,8 +1593,15 @@ static void show_dirstat(struct diff_options *options)
* Original minus copied is the removed material,
* added is the new material. They are both damages
* 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;
if (!damage)
damage = 1;

found_damage:
ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);
Expand Down
2 changes: 0 additions & 2 deletions t/t4013-diff-various.sh
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,7 @@ diff --no-index --name-status -- dir2 dir
diff --no-index dir dir3
diff master master^ side
diff --dirstat master~1 master~2
# --dirstat doesn't notice changes that simply rearrange existing lines
diff --dirstat initial rearrange
# ...but --dirstat-by-file does notice changes that only rearrange lines
diff --dirstat-by-file initial rearrange
EOF

Expand Down
1 change: 1 addition & 0 deletions t/t4013/diff.diff_--dirstat_initial_rearrange
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
$ git diff --dirstat initial rearrange
100.0% dir/
$

0 comments on commit 2ff3a80

Please sign in to comment.