On Thu, Jan 14, 2016 at 09:29:49PM -0500, Thomas Dickey wrote: > On Sat, Jan 02, 2016 at 06:46:52PM +0000, Sandro Tosi wrote: > > Adding Thomas, the upstream author of diffstat > > > > Hey Thomas, > > what's your take on this report? > > > > On Sun, Aug 2, 2015 at 6:39 PM, Josh Triplett <[email protected]> wrote: > > > On Sat, Aug 01, 2015 at 06:33:39PM -0700, Josh Triplett wrote: > > >> If the diff contains files with very large diffs, diffstat's automatic > > >> scaling can cause files with small diffs to display zero '-' or '+' > > >> characters. This hides key information from the diffstat, namely > > >> the direction of the diff. diffstat should always display at least one > > >> '-' for a file with lines removed, and at least one '+' for a file with > > >> lines added, regardless of scaling. > > > > > > Steps to reproduce: > > > > > > /tmp$ mkdir foo > > > /tmp$ cd foo/ > > > /tmp/foo$ mkdir 1 > > > /tmp/foo$ mkdir 2 > > > /tmp/foo$ echo one-line > 2/only-in-2 > > > /tmp/foo$ echo one-line > 1/only-in-1 > > > /tmp/foo$ seq 1 100000 > 2/big-file > > > /tmp/foo$ diff -Naur 1 2 | diffstat > > If it used > > /tmp/foo$ diff -Naur 1 2 | diffstat -r2 > > > > big-file |100000 > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > > only-in-1 | 1 > > > only-in-2 | 1 > > > 3 files changed, 100001 insertions(+), 1 deletion(-) > > the output would be > > big-file |100000 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > only-in-1 | 1 - > only-in-2 | 1 + > 3 files changed, 100001 insertions(+), 1 deletion(-) > > man diffstat: > > -r code > provides optional rounding of the data shown in histogram, > rather than truncating with error adjustments. > > 0 is the default. No rounding is performed, but accumulated > errors are added to following columns. > > 1 rounds the data > > 2 rounds the data and adjusts the histogram to ensure that it > displays something if there are any differences even if those > would normally be rounded to zero.
Interesting; I didn't know about that option. However, it doesn't seem like any of the three options do the right thing in all cases. -r2 handles the above case correctly, but not this one: /tmp$ mkdir 1 2 /tmp$ seq 1 10000 > 2/big-file /tmp$ echo foo > 1/big-file /tmp$ diff -Naur 1 2 | diffstat -r2 big-file |10001 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 10000 insertions(+), 1 deletion(-) Note the lack of '-' at the end. The default -r0 displays the '-' in this case, but doesn't handle the above case. The default behavior of "git diff --no-index --stat" handles all of those cases, always displaying at least one '-' or '+' if a change of that type occurred. So, I'd still request a new rounding mode that handles all of those cases, since the current rounding modes only handle a subset of cases, and git provides an existence proof for an algorithm that handles all of them. I'd also argue for making that new rounding mode the default. It seems exceedingly unlikely that anything would both 1) parse the output of diffstat and 2) depend on the *exact* number of '+' and '-' signs printed by diffstat. So I don't see any chance of such a change breaking anything. - Josh Triplett
