One of the common kind of whitespace errors is to lack the final newline at the end of a file, but so far, neither "git diff" or "git apply" did anything about them.
This series introduces "incomplete-line" whitespace error class, that you can add to either the core.whitespace configuration variable, or the whitespace attribute in your .gitattributes files.
The class is disabled by default, so the final step enables it for our project by defining it in the .gitattributes file.
The incomplete line marker that is given for a context line is not considered an error. The reasoning is that your preimage did have incomplete line, but you did not touch the contents on that incomplete line in your patch, so you left the line intact. It is not a new breakage you are responsible for.
If the incomplete line marker follows a postimage line, on the other hand, it means that you added a new line at the end of the file that is incomplete *and* that line did not exist in the preimage. The last line of the preimage may have been incomplete already, but then you updated the contents on that line, so you could have easily fixed the incompleteness of the line while at it. Either way, you are responsible for the incompleteness of the last ine in the resulting file.
The organization of the series is as follows.
* The first patch [01/12] is a preliminary clean-up to reformat the
definition of bit assignments for whitespace error classes.
* The patches [02/12] - [08/12] are preliminary clean-up made to
both "git diff" and "git apply" machinery.
* The patch [09/12] shifts the bit assignment to make room for new
whitespace error class (which was last updated in 2007 IIRC, so
the set of whitespace errors surprisingly haven't changed for
quite some time), and defines the new "incomplete-line" class.
* The patch [10/12] teaches "git apply --whitespace=<mode>" and
"git apply --check" about the incomplete-line error class.
* The patch [11/12] teaches "git diff [--check]" about the
incomplete-line error class.
* The final patch [12/12] enables the incomplete-line error class
for our project for C source files and shell scripts. I didn't
touch the cover-all * entry.
Changes in v4:
- The preliminary clean-up for "git diff" in [04/12] no longer
"corrects" the code that updates the line number upon seeing
"\ No newline" marker, and leaves it for later series to clean
it up as #leftoverbits.
- Our house rule updates in [12/12] now forbids the documentation
sources *.adoc to end in an incomplete line.
- Some typofixes in log messages.
Changes in v3:
- The proposed log message of [PATCH 05/12] explains that it
semi-duplicates the same code shared in two case arems in
preparation for later changes.
- The internal helper function to emit the "\ No newline" marker
line is now called emit_incomplete_line_marker().
- Two conditionals in [PATCH 07/12] both of which switched on !endp
have been consolidated into a single if/else statement.
- The tests in [PATCH 11/12] checks the output from "diff --check"
now.
Changes in v2:
- rolled the definition (but not implementation) of the new
"incomplete-line" class into step [09/12] that shifts the bit
assignment. The documentation of core.whitespace has also be
updated in this step.
- "git apply --check" miscounted line number reported for the
incomplete line error, which has been corrected in step [10/12].
- t4124-apply-ws-rule.sh has been extended to cover "git apply
--check" and the diagnostic output from it in step [10/12].
Junio C Hamano (12):
whitespace: correct bit assignment comments
diff: emit_line_ws_markup() if/else style fix
diff: correct suppress_blank_empty hack
diff: keep track of the type of the last line seen
diff: refactor output of incomplete line
diff: call emit_callback ecbdata everywhere
diff: update the way rewrite diff handles incomplete lines
apply: revamp the parsing of incomplete lines
whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE
apply: check and fix incomplete lines
diff: highlight and error out on incomplete lines
attr: enable incomplete-line whitespace error for this project
.gitattributes | 6 +-
Documentation/config/core.adoc | 2 +
apply.c | 79 ++++++++++----
diff.c | 145 ++++++++++++++++---------
diff.h | 6 +-
t/t4015-diff-whitespace.sh | 67 +++++++++++-
t/t4124-apply-ws-rule.sh | 187 +++++++++++++++++++++++++++++++++
ws.c | 20 ++++
ws.h | 26 +++--
9 files changed, 450 insertions(+), 88 deletions(-)
1: 39ce594b48 = 1: 5486212662 whitespace: correct bit assignment comments
2: 7c53ac154b = 2: 89504329a4 diff: emit_line_ws_markup() if/else style fix
3: 239644a8b4 = 3: 78f55aebf6 diff: correct suppress_blank_empty hack
4: a2c279cca0 ! 4: 9f6b6ad756 diff: fix incorrect counting of line numbers
@@ Metadata
Author: Junio C Hamano <gitster@pobox.com>
## Commit message ##
- diff: fix incorrect counting of line numbers
+ diff: keep track of the type of the last line seen
The "\ No newline at the end of the file" can come after any of the
"-" (deleted preimage line), " " (unchanged line), or "+" (added
- postimage line). Incrementing only the preimage line number upon
- seeing it does not make any sense.
+ postimage line). In later steps in this series, we will start
+ treating a change that makes a file to end in an incomplete line
+ as a whitespace error, and we would need to know what the previous
+ line was when we react to "\ No newline" in the diff output. If
+ the previous line was a context (i.e., unchanged) line, the file
+ lacked the final newline before the change, and the change did not
+ touch that line and left it still incomplete, so we do not want to
+ warn in such a case.
- We can keep track of what the previous line was, and increment
- lno_in_{pre,post}image variables properly, like this patch does. I
- do not think it matters, as these numbers are used only to compare
- them with blank_at_eof_in_{pre,post}image to issue the warning every
- time we see an added line, but by definition, after we see "\ No
- newline at the end of the file" for an added line, we will not see
- an added line for the file.
+ Teach fn_out_consume() function to keep track of what the previous
+ line was, and prepare an otherwise empty switch statement to let us
+ react differently to "\ No newline" based on that.
- Keeping track of what the last line was (in other words, "is it that
- the file used to end in an incomplete line? The file ends in an
- incomplete line after the change? Both the file before and after
- the change ends in an incomplete line that did not change?") will be
- independently useful.
+ Note that there is an existing curiosity (read: likely to be a bug)
+ in the code that increments line number in the preimage file every
+ time it sees a line with "\ No newline" on it, regardless of what
+ the previous line was. I left it as-is, because it does not affect
+ the main theme of this series, and more importantly, I do not think
+ it matters, as these numbers are used only to compare them with
+ blank_at_eof_in_{pre,post}image to issue a warning when we see more
+ empty line was added at the end, but by definition, after we see
+ "\ No newline at the end of the file" for an added line, we will not
+ see an added line for the file.
+
+ An independent audit to ensure that this curious increment can be
+ safely removed would make a good #leftoverbits clean-up (we may even
+ find some code that decrements this counter or over-increments the
+ other quantity this counter is compared with that compensates the
+ effect of this curious increment that hides a bug, in which case we
+ may also need to remove them).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@@ diff.c: static int fn_out_consume(void *priv, char *line, unsigned long len)
break;
case '\\':
/* incomplete line at the end */
-- ecbdata->lno_in_preimage++;
+ switch (ecbdata->last_line_kind) {
+ case '+':
-+ ecbdata->lno_in_postimage++;
-+ break;
+ case '-':
-+ ecbdata->lno_in_preimage++;
-+ break;
+ case ' ':
-+ ecbdata->lno_in_preimage++;
-+ ecbdata->lno_in_postimage++;
+ break;
+ default:
+ BUG("fn_out_consume: '\\No newline' after unknown line (%c)",
+ ecbdata->last_line_kind);
+ }
+ ecbdata->lno_in_preimage++;
emit_diff_symbol(o, DIFF_SYMBOL_CONTEXT_INCOMPLETE,
line, len, 0);
- break;
+@@ diff.c: static int fn_out_consume(void *priv, char *line, unsigned long len)
default:
BUG("fn_out_consume: unknown line '%s'", line);
}
5: 7a91bf04be ! 5: 16dda4fe13 diff: refactor output of incomplete line
@@ Commit message
Create a helper function that reacts to "\ No newline at the end of
file" in preparation for unifying the incomplete line handling in
the code path that handles xdiff output and the code path that
- bypasses xdiff and produces complete rewrite patch.
+ bypasses xdiff and produces a complete-rewrite patch.
Currently the output from the DIFF_SYMBOL_CONTEXT_INCOMPLETE case
still (ab)uses the same code as what is used for context lines, but
- that would change in a later step where we introduce support for
- incomplete line detection.
+ that would change in a later step where we introduce support to treat
+ an incomplete line as a whitespace error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@@ diff.c: static void emit_context_line(struct emit_callback *ecbdata,
const char *line, int len)
{
@@ diff.c: static int fn_out_consume(void *priv, char *line, unsigned long len)
- BUG("fn_out_consume: '\\No newline' after unknown line (%c)",
ecbdata->last_line_kind);
}
+ ecbdata->lno_in_preimage++;
- emit_diff_symbol(o, DIFF_SYMBOL_CONTEXT_INCOMPLETE,
- line, len, 0);
+ emit_incomplete_line_marker(ecbdata, line, len);
6: 3f56e084f8 = 6: dd310cc6dc diff: call emit_callback ecbdata everywhere
7: af5b4c94e4 = 7: 3360f2882c diff: update the way rewrite diff handles incomplete lines
8: e4945e8951 = 8: ff29d34f3c apply: revamp the parsing of incomplete lines
9: d4fd89039a = 9: b720c4f49d whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE
10: 075ef5d32f = 10: 5ddf90cd08 apply: check and fix incomplete lines
11: 04401decef = 11: cf52c68ec1 diff: highlight and error out on incomplete lines
12: 068229790d ! 12: cb86d9b45f attr: enable incomplete-line whitespace error for this project
@@ Commit message
Now "git diff --check" and "git apply --whitespace=warn/fix" learned
incomplete line is a whitespace error, enable them for this project
- to prevent patches to add new incomplete lines to our sources.
+ to prevent patches to add new incomplete lines to our source to both
+ code and documentation files.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@@ .gitattributes
*.perl text eol=lf diff=perl
*.pl text eof=lf diff=perl
*.pm text eol=lf diff=perl
+ *.py text eol=lf diff=python
+ *.bat text eol=crlf
+ CODE_OF_CONDUCT.md -whitespace
+-/Documentation/**/*.adoc text eol=lf
++/Documentation/**/*.adoc text eol=lf whitespace=!indent,trail,space,incomplete
+ /command-list.txt text eol=lf
+ /GIT-VERSION-GEN text eol=lf
+ /mergetools/* text eol=lf
Junio C Hamano
