Junio C Hamano <gits...@pobox.com> writes:
> It is pleasing to see that with a surprisingly clean and small
> change like this we can exempt the initial space byte from
> SP-before-HT check and from Indent-with-non-tab at the same time.
>
> Very nice.
>
> One reason why a surprisingly small special case is required is
> perhaps because we are blessed with the original code being clean
> [*1*], and the fact that a line[0] that is not ' ' will not trigger
> any indentation related whitespace errors without this special case,
> I guess.
Having said good things about the patch, I unfortunately realized
that we weren't that lucky. As we do want to see whitespace errors
on lines with line[0] != ' ' to be flagged. So "... will not
trigger" in the above is not a blessing, but something that further
needs to be fixed. The special case should also be made for line[0]
that is '+' and possibly '-' (and I also suspect that the changes in
this patch may mostly be reusable with little tweaks if any).
Imagine we start from this commit that "git show" shows us like so:
int main(int ac, char **av)
{
________ printf("Hello");
+________ putchar(',');
+ putchar(' ');
printf("World\n");
return 0;
}
I've drawn a horizontal-tab as long underscore to clarify in the
above picture. If you have "core.whitespace=indent-with-non-tab"
"git show" would paint the line that adds " " as violating (as it
types 10 SPs, when it could have been a tab and 2 SPs), but it does
not highlight the line with "World" or "return", which is sensible
as they are pre-existing violations.
Then imagine we did "git commit --amend" and "git show" would give
this instead:
int main(int ac, char **av)
{
________ printf("Hello");
+ putchar(',');
+________ putchar(' ');
printf("World\n");
return 0;
}
That is, relative to the previous attempt, we stopped introducing
new indent-with-non-tab violation to the line that adds " ", but
added a new violation to the line that adds ",".
After such "git commit --amend", what do we want to see in the
output from "git range-diff @{1}..."?
My quick test of the current code does not show any whitespace
breakage for either versions. I *think* what we want to actually
see is
- just like WS_IGNORE_FIRST_SPACE logic shifted the column by
incrementing written (and final condition) for the loop in your
patch for line[0]==' ', detect the overlong run of SP correctly
by ignoring the first column that is '+', and complaining that
the new commit is typing 10 SPs before putchar(',').
- Earlier I thought that lines with '-' in the outer diff should
become exempt from whitespace-error highlighting, but I think
that is a mistake. If a line in diff-of-diff that begins with
"-+" has whitespace violation (e.g "-+" followed by 10 SPs), that
is "the old version of the patch used to introduce whitespace
violation", which is a useful piece of information when we look
at the corresponding line in the same diff-of-diff that begins
with "++". We could either say "and you cleaned that mistake up
in your newer patch", or "and you still have that mistake in your
newer patch" (another possibility is there is no whitespace error
on a "-+" line, and the corresponding "++" line has one---"you
got it right in the previous round, but you somehow made it
worse").
Here is the reproduction of the sample data I based on the above
thought experiment on.
diff --git a/script.sh b/script.sh
new file mode 100755
index 0000000..0090661
--- /dev/null
+++ b/script.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+git init
+git config core.whitespace indent-with-non-tab
+
+tr _ '\011' <<\EOF >hello.c
+int main(int ac, char **av)
+{
+_ printf("Hello");
+ printf("World\n");
+ return 0;
+}
+EOF
+
+git add hello.c && git commit -m initial
+
+tr _ '\011' <<\EOF >hello.c
+int main(int ac, char **av)
+{
+_ printf("Hello");
+ putchar(',');
+_ putchar(' ');
+ printf("World\n");
+ return 0;
+}
+EOF
+
+git commit -a -m second
+
+tr _ '\011' <<\EOF >hello.c
+int main(int ac, char **av)
+{
+_ printf("Hello");
+_ putchar(',');
+ putchar(' ');
+ printf("World\n");
+ return 0;
+}
+EOF
+
+git commit -a --amend -m third
+
+
+git show @{1}
+git show HEAD
+
+git range-diff ..@{1} @{1}..
+
--
2.18.0-232-gb7bd9486b0
