aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--builtin-apply.c5
-rw-r--r--cache.h6
-rw-r--r--diff.c95
-rwxr-xr-xt/t4015-diff-whitespace.sh6
-rwxr-xr-xt/t4017-diff-retval.sh14
-rwxr-xr-xtemplates/hooks--pre-commit.sample64
-rw-r--r--ws.c33
7 files changed, 136 insertions, 87 deletions
diff --git a/builtin-apply.c b/builtin-apply.c
index 318504a03..985ca3be5 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -987,8 +987,7 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
static void check_whitespace(const char *line, int len, unsigned ws_rule)
{
char *err;
- unsigned result = check_and_emit_line(line + 1, len - 1, ws_rule,
- NULL, NULL, NULL, NULL);
+ unsigned result = ws_check(line + 1, len - 1, ws_rule);
if (!result)
return;
@@ -999,7 +998,7 @@ static void check_whitespace(const char *line, int len, unsigned ws_rule)
else {
err = whitespace_error_string(result);
fprintf(stderr, "%s:%d: %s.\n%.*s\n",
- patch_input_file, linenr, err, len - 2, line + 1);
+ patch_input_file, linenr, err, len - 2, line + 1);
free(err);
}
}
diff --git a/cache.h b/cache.h
index 64ef86e12..188428dd2 100644
--- a/cache.h
+++ b/cache.h
@@ -819,11 +819,11 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
extern unsigned whitespace_rule_cfg;
extern unsigned whitespace_rule(const char *);
extern unsigned parse_whitespace_rule(const char *);
-extern unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
- FILE *stream, const char *set,
- const char *reset, const char *ws);
+extern unsigned ws_check(const char *line, int len, unsigned ws_rule);
+extern void ws_check_emit(const char *line, int len, unsigned ws_rule, FILE *stream, const char *set, const char *reset, const char *ws);
extern char *whitespace_error_string(unsigned ws);
extern int ws_fix_copy(char *, const char *, int, unsigned, int *);
+extern int ws_blank_line(const char *line, int len, unsigned ws_rule);
/* ls-files */
int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);
diff --git a/diff.c b/diff.c
index 66851b564..803fbba45 100644
--- a/diff.c
+++ b/diff.c
@@ -535,9 +535,9 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
else {
/* Emit just the prefix, then the rest. */
emit_line(ecbdata->file, set, reset, line, ecbdata->nparents);
- (void)check_and_emit_line(line + ecbdata->nparents,
- len - ecbdata->nparents, ecbdata->ws_rule,
- ecbdata->file, set, reset, ws);
+ ws_check_emit(line + ecbdata->nparents,
+ len - ecbdata->nparents, ecbdata->ws_rule,
+ ecbdata->file, set, reset, ws);
}
}
@@ -1136,42 +1136,85 @@ static void free_diffstat_info(struct diffstat_t *diffstat)
struct checkdiff_t {
struct xdiff_emit_state xm;
const char *filename;
- int lineno, color_diff;
+ int lineno;
+ struct diff_options *o;
unsigned ws_rule;
unsigned status;
- FILE *file;
+ int trailing_blanks_start;
};
+static int is_conflict_marker(const char *line, unsigned long len)
+{
+ char firstchar;
+ int cnt;
+
+ if (len < 8)
+ return 0;
+ firstchar = line[0];
+ switch (firstchar) {
+ case '=': case '>': case '<':
+ break;
+ default:
+ return 0;
+ }
+ for (cnt = 1; cnt < 7; cnt++)
+ if (line[cnt] != firstchar)
+ return 0;
+ /* line[0] thru line[6] are same as firstchar */
+ if (firstchar == '=') {
+ /* divider between ours and theirs? */
+ if (len != 8 || line[7] != '\n')
+ return 0;
+ } else if (len < 8 || !isspace(line[7])) {
+ /* not divider before ours nor after theirs */
+ return 0;
+ }
+ return 1;
+}
+
static void checkdiff_consume(void *priv, char *line, unsigned long len)
{
struct checkdiff_t *data = priv;
- const char *ws = diff_get_color(data->color_diff, DIFF_WHITESPACE);
- const char *reset = diff_get_color(data->color_diff, DIFF_RESET);
- const char *set = diff_get_color(data->color_diff, DIFF_FILE_NEW);
+ int color_diff = DIFF_OPT_TST(data->o, COLOR_DIFF);
+ const char *ws = diff_get_color(color_diff, DIFF_WHITESPACE);
+ const char *reset = diff_get_color(color_diff, DIFF_RESET);
+ const char *set = diff_get_color(color_diff, DIFF_FILE_NEW);
char *err;
if (line[0] == '+') {
unsigned bad;
data->lineno++;
- bad = check_and_emit_line(line + 1, len - 1,
- data->ws_rule, NULL, NULL, NULL, NULL);
+ if (!ws_blank_line(line + 1, len - 1, data->ws_rule))
+ data->trailing_blanks_start = 0;
+ else if (!data->trailing_blanks_start)
+ data->trailing_blanks_start = data->lineno;
+ if (is_conflict_marker(line + 1, len - 1)) {
+ data->status |= 1;
+ fprintf(data->o->file,
+ "%s:%d: leftover conflict marker\n",
+ data->filename, data->lineno);
+ }
+ bad = ws_check(line + 1, len - 1, data->ws_rule);
if (!bad)
return;
data->status |= bad;
err = whitespace_error_string(bad);
- fprintf(data->file, "%s:%d: %s.\n", data->filename, data->lineno, err);
+ fprintf(data->o->file, "%s:%d: %s.\n",
+ data->filename, data->lineno, err);
free(err);
- emit_line(data->file, set, reset, line, 1);
- (void)check_and_emit_line(line + 1, len - 1, data->ws_rule,
- data->file, set, reset, ws);
- } else if (line[0] == ' ')
+ emit_line(data->o->file, set, reset, line, 1);
+ ws_check_emit(line + 1, len - 1, data->ws_rule,
+ data->o->file, set, reset, ws);
+ } else if (line[0] == ' ') {
data->lineno++;
- else if (line[0] == '@') {
+ data->trailing_blanks_start = 0;
+ } else if (line[0] == '@') {
char *plus = strchr(line, '+');
if (plus)
data->lineno = strtol(plus, NULL, 10) - 1;
else
die("invalid diff");
+ data->trailing_blanks_start = 0;
}
}
@@ -1544,8 +1587,9 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
static void builtin_checkdiff(const char *name_a, const char *name_b,
const char *attr_path,
- struct diff_filespec *one,
- struct diff_filespec *two, struct diff_options *o)
+ struct diff_filespec *one,
+ struct diff_filespec *two,
+ struct diff_options *o)
{
mmfile_t mf1, mf2;
struct checkdiff_t data;
@@ -1557,13 +1601,18 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
data.xm.consume = checkdiff_consume;
data.filename = name_b ? name_b : name_a;
data.lineno = 0;
- data.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
+ data.o = o;
data.ws_rule = whitespace_rule(attr_path);
- data.file = o->file;
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
+ /*
+ * All the other codepaths check both sides, but not checking
+ * the "old" side here is deliberate. We are checking the newly
+ * introduced changes, and as long as the "new" side is text, we
+ * can and should check what it introduces.
+ */
if (diff_filespec_is_binary(two))
goto free_and_return;
else {
@@ -1577,6 +1626,12 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
ecb.outf = xdiff_outf;
ecb.priv = &data;
xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
+
+ if (data.trailing_blanks_start) {
+ fprintf(o->file, "%s:%d: ends with blank lines.\n",
+ data.filename, data.trailing_blanks_start);
+ data.status = 1; /* report errors */
+ }
}
free_and_return:
diff_free_filespec_data(one);
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index b7cc6b28e..0922c708f 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -335,4 +335,10 @@ test_expect_success 'line numbers in --check output are correct' '
'
+test_expect_success 'checkdiff detects trailing blank lines' '
+ echo "foo();" >x &&
+ echo "" >>x &&
+ git diff --check | grep "ends with blank"
+'
+
test_done
diff --git a/t/t4017-diff-retval.sh b/t/t4017-diff-retval.sh
index 0d0fb87f5..60dd2014d 100755
--- a/t/t4017-diff-retval.sh
+++ b/t/t4017-diff-retval.sh
@@ -113,4 +113,18 @@ test_expect_success 'check should test not just the last line' '
'
+test_expect_success 'check detects leftover conflict markers' '
+ git reset --hard &&
+ git checkout HEAD^ &&
+ echo binary >>b &&
+ git commit -m "side" b &&
+ test_must_fail git merge master &&
+ git add b && (
+ git --no-pager diff --cached --check >test.out
+ test $? = 2
+ ) &&
+ test 3 = $(grep "conflict marker" test.out | wc -l) &&
+ git reset --hard
+'
+
test_done
diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index 71c10f25f..0e49279c7 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -7,64 +7,12 @@
#
# To enable this hook, rename this file to "pre-commit".
-# This is slightly modified from Andrew Morton's Perfect Patch.
-# Lines you introduce should not have trailing whitespace.
-# Also check for an indentation that has SP before a TAB.
-
if git-rev-parse --verify HEAD 2>/dev/null
then
- git-diff-index -p -M --cached HEAD --
+ against=HEAD
else
- # NEEDSWORK: we should produce a diff with an empty tree here
- # if we want to do the same verification for the initial import.
- :
-fi |
-perl -e '
- my $found_bad = 0;
- my $filename;
- my $reported_filename = "";
- my $lineno;
- sub bad_line {
- my ($why, $line) = @_;
- if (!$found_bad) {
- print STDERR "*\n";
- print STDERR "* You have some suspicious patch lines:\n";
- print STDERR "*\n";
- $found_bad = 1;
- }
- if ($reported_filename ne $filename) {
- print STDERR "* In $filename\n";
- $reported_filename = $filename;
- }
- print STDERR "* $why (line $lineno)\n";
- print STDERR "$filename:$lineno:$line\n";
- }
- while (<>) {
- if (m|^diff --git a/(.*) b/\1$|) {
- $filename = $1;
- next;
- }
- if (/^@@ -\S+ \+(\d+)/) {
- $lineno = $1 - 1;
- next;
- }
- if (/^ /) {
- $lineno++;
- next;
- }
- if (s/^\+//) {
- $lineno++;
- chomp;
- if (/\s$/) {
- bad_line("trailing whitespace", $_);
- }
- if (/^\s* \t/) {
- bad_line("indent SP followed by a TAB", $_);
- }
- if (/^([<>])\1{6} |^={7}$/) {
- bad_line("unresolved merge conflict", $_);
- }
- }
- }
- exit($found_bad);
-'
+ # Initial commit: diff against an empty tree object
+ against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+fi
+
+exec git diff-index --check --cached $against --
diff --git a/ws.c b/ws.c
index ba7e834ca..7a7ff130a 100644
--- a/ws.c
+++ b/ws.c
@@ -117,9 +117,9 @@ char *whitespace_error_string(unsigned ws)
}
/* If stream is non-NULL, emits the line after checking. */
-unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
- FILE *stream, const char *set,
- const char *reset, const char *ws)
+static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,
+ FILE *stream, const char *set,
+ const char *reset, const char *ws)
{
unsigned result = 0;
int written = 0;
@@ -213,6 +213,33 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
return result;
}
+void ws_check_emit(const char *line, int len, unsigned ws_rule,
+ FILE *stream, const char *set,
+ const char *reset, const char *ws)
+{
+ (void)ws_check_emit_1(line, len, ws_rule, stream, set, reset, ws);
+}
+
+unsigned ws_check(const char *line, int len, unsigned ws_rule)
+{
+ return ws_check_emit_1(line, len, ws_rule, NULL, NULL, NULL, NULL);
+}
+
+int ws_blank_line(const char *line, int len, unsigned ws_rule)
+{
+ /*
+ * We _might_ want to treat CR differently from other
+ * whitespace characters when ws_rule has WS_CR_AT_EOL, but
+ * for now we just use this stupid definition.
+ */
+ while (len-- > 0) {
+ if (!isspace(*line))
+ return 0;
+ line++;
+ }
+ return 1;
+}
+
/* Copy the line to the buffer while fixing whitespaces */
int ws_fix_copy(char *dst, const char *src, int len, unsigned ws_rule, int *error_count)
{