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>

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 23772d615d22e0a4bbad41e82e156634981d23c3..7e4bd425e1fec23bbca0e89a0d7b4f53cf4f4483 100644 (file)
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -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
        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.
 
 --dirstat-by-file[=<limit>]::
        Same as `--dirstat`, but counts changed files instead of lines.

diff --git a/diff.c b/diff.c
index 4f5270b8dbc05c820c24075b499fe26afacf52a8..1f44cb4237317fd2e36bc9b00047b67f7e844e17 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1548,6 +1548,16 @@ static void show_dirstat(struct diff_options *options)
                else
                        content_changed = 1;
 
                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
                if (DIFF_OPT_TST(options, DIRSTAT_BY_FILE)) {
                        /*
                         * 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
                         * (with each file contributing equal damage).
                         */
                         * add this file to the list of results
                         * (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,
                 * added is the new material.  They are both damages
                 * made to the preimage.
                 * 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;
                 */
                damage = (p->one->size - copied) + added;

+               if (!damage)
+                       damage = 1;

 
 found_damage:
                ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);
 
 found_damage:
                ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);

diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 6428a905ab76696c6d7d9e20084b3368ac855a14..93a6f208710befc064b7b99bcd758bb8b6381918 100755 (executable)
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -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
 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
 diff --dirstat initial rearrange

-# ...but --dirstat-by-file does notice changes that only rearrange lines

 diff --dirstat-by-file initial rearrange
 EOF
 
 diff --dirstat-by-file initial rearrange
 EOF
 

diff --git a/t/t4013/diff.diff_--dirstat_initial_rearrange b/t/t4013/diff.diff_--dirstat_initial_rearrange
index fb2e17dd2e48f5b4bc0a03ca274156ffeb21782d..5fb02c13bc51021aa6405a2271fce150432b6ce8 100644 (file)
--- a/t/t4013/diff.diff_--dirstat_initial_rearrange
+++ b/t/t4013/diff.diff_--dirstat_initial_rearrange
@@ -1,2 +1,3 @@
 $ git diff --dirstat initial rearrange
 $ git diff --dirstat initial rearrange

+ 100.0% dir/

 $
 $