Re: [PATCH 1/3 v4] diff --stat: use the full terminal width

Zbigniew Jędrzejewski-Szmek [off-list ref] writes:

- comments are updated and the word "histogram" is banished

Heh, I still see at least three instances of them in this patch.

- use decimal_width(max_change) to calculate number of columns

Please see comments for 3/3 I'll send separately.

How long is this name?  120 columns?  I think that should be fine for any
filesystem we care about.

+test_expect_success 'preparation' "
+	> ${name} &&
+	git add ${name} &&
+	git commit -m message &&
+	echo a > ${name} &&
+	git commit -m message ${name}
+"

Please write these (exactly -- paying close attention to SP) like this:

	>"$name" &&
	git add "$name" &&
        git commit -m message &&
        echo a >"$name" &&
        git commit -m message "$name"

Points to note:

 - Even though you (and the reader) may know that "$name" does not contain
   word-breaking spaces, writing double-quotes around it reduces the
   mental burden from the readers;

 - Strictly speaking, the target of I/O redirection (e.g. >"$name") does
   not have to have quotes around it, but some versions of bash are known
   to give misguided warnings against it;

 - We do not write SP between the redirection and filename, but we do have
   one SP before the redirection; and

 - Unless you want to express concatenation with string that can appear
   later in words (e.g. "${name}1"), avoid ${name} to lessen the mental
   burden to readers, as "${" often signals there is some magic coming
   (e.g. "${name#strip}").

+cat >expect <<'EOF'
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +
+EOF
+test_expect_success 'format patch graph width is 80 columns' '

Please make sure this test (and all the other new tests) pass with or
without your the patch that updates diff.c, as we need to ensure that
COLUMNS=80 or vanilla format-patch produces result that is identical to
the old output without regression (again, see comments to [PATCH 3/3]).

+	git format-patch --stat --stdout -1 |
+		grep -m 1 aaaaa > actual &&

Do not use "grep -m $count"; it is not portable.

Literal translation of the above would be:

	sed -n -e "/aaaaa/{
        	p
                q
	}"

but you can perhaps rely on the fact that there is only one path and '|'
does not ppear in the payload or log message, and use this instead:

	grep "|" >actual

Also to catch errors in format-patch that unexpectedly dies (after all,
you are touching diff machinery with your patch), avoid using pipes, and
write it perhaps like this:

	git format-patch --stat --stdout -1 >output &&
        grep "|" >actual &&