Fix git diff --stat for interesting - but empty - file changes

2012-10-17 Thread Linus Torvalds
The behavior of git diff --stat is rather odd for files that have
zero lines of changes: it will discount them entirely unless they were
renames.

Which means that the stat output will simply not show files that only
had other changes: they were created or deleted, or their mode was
changed.

Now, those changes do show up in the summary, but so do renames, so
the diffstat logic is inconsistent. Why does it show renames with zero
lines changed, but not mode changes or added files with zero lines
changed?

So change the logic to not check for is_renamed, but for
is_interesting instead, where interesting is judged to be any
action but a pure data change (because a pure data change with zero
data changed really isn't worth showing, if we ever get one in our
diffpairs).

So if you did

   chmod +x Makefile
   git diff --stat

before, it would show empty ( 0 files changed), with this it shows

 Makefile | 0
 1 file changed, 0 insertions(+), 0 deletions(-)

which I think is a more correct diffstat (and then with --summary it
shows *what* the metadata change to Makefile was - this is completely
consistent with our handling of renamed files).

Side note: the old behavior was *really* odd. With no changes at all,
git diff --stat output was empty. With just a chmod, it said 0
files changed. No way is our legacy behavior sane.

Signed-off-by: Linus Torvalds torva...@linux-foundation.org
---

This was triggered by kernel developers not noticing that they had
added zero-sized files, because those additions never showed up in the
diffstat.

NOTE! This does break two of our tests, so we clearly did this on
purpose, or at least tested for it. I just uncommented the subtests
that this makes irrelevant, and changed the output of another one.

Another test was simply buggy. It used git diff --root cmit, and
thought that would be the diff against root. It isn't, and never has
been. It just happened to give the same (no file) output before.
Fixing --stat to show new files showed how buggy the test was. The
--root thing matters for git show or git log (when showing a
root commit) and for git diff-tree with a single tree.

Maybe we would *want* to make git diff --root cmit be the diff
between root and cmit, but that's not what it actually is.

Comments?


patch.diff
Description: Binary data


Re: Fix git diff --stat for interesting - but empty - file changes

2012-10-17 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 So if you did

chmod +x Makefile
git diff --stat

 before, it would show empty ( 0 files changed), with this it shows

  Makefile | 0
  1 file changed, 0 insertions(+), 0 deletions(-)

 which I think is a more correct diffstat (and then with --summary it
 shows *what* the metadata change to Makefile was - this is completely
 consistent with our handling of renamed files).

 Side note: the old behavior was *really* odd. With no changes at all,
 git diff --stat output was empty. With just a chmod, it said 0
 files changed. No way is our legacy behavior sane.

 Signed-off-by: Linus Torvalds torva...@linux-foundation.org
 ---

 This was triggered by kernel developers not noticing that they had
 added zero-sized files, because those additions never showed up in the
 diffstat.
 ...
 Comments?

I think listing a file whose content remain unchanged with 0 as the
number of lines affected makes sense, and it will mesh well with
Duy's

  http://thread.gmane.org/gmane.comp.version-control.git/207749

I first wondered if we would get a division-by-zero while scaling
the graph, but we do not scale smaller numbers up to fill the
columns, so we should be safe.

These days, we omit 0 insertions and 0 deletions, so I am not sure
what you should get for this case, though:

  Makefile | 0
  1 file changed, 0 insertions(+), 0 deletions(-)

Should we just say 1 file changed?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fix git diff --stat for interesting - but empty - file changes

2012-10-17 Thread Linus Torvalds
On Wed, Oct 17, 2012 at 11:28 AM, Junio C Hamano gits...@pobox.com wrote:

 I think listing a file whose content remain unchanged with 0 as the
 number of lines affected makes sense, and it will mesh well with
 Duy's

   http://thread.gmane.org/gmane.comp.version-control.git/207749

 I first wondered if we would get a division-by-zero while scaling
 the graph, but we do not scale smaller numbers up to fill the
 columns, so we should be safe.

Note that we should be safe for a totally different - and more
fundamental - reason: the zero line case is by no means new. We've
always done it for the rename case.

 These days, we omit 0 insertions and 0 deletions, so I am not sure
 what you should get for this case, though:

  Makefile | 0
  1 file changed, 0 insertions(+), 0 deletions(-)

 Should we just say 1 file changed?

If that is what it does for the rename case, then yes. I think it
should fall out naturally.

  Linus
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html