"diff --check" should affect exit status

"git diff" has a --check option that can be used to check for whitespace
problems but it only reported by printing warnings to the
console.

Now when the --check option is used we give a non-zero exit status,
making "git diff --check" nicer to use in scripts and hooks.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 5d22b7b58c5950fe70d09e0bb7384179ae30220f..9ecc1d7bc460dd723ae08618b0deab88e407114f 100644 (file)
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -93,7 +93,9 @@ endif::git-format-patch[]
 
 --check::
        Warn if changes introduce trailing whitespace
-       or an indent that uses a space before a tab.
+       or an indent that uses a space before a tab. Exits with
+       non-zero status if problems are found. Not compatible with
+       --exit-code.
 
 --full-index::
        Instead of the first handful characters, show full

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 046b7e34b5d5c6306d335dddbd5e283c8166e80e..4afc8724e71449667ce22ac8efd0910b7a57ef42 100644 (file)
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -33,5 +33,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
        result = run_diff_files_cmd(&rev, argc, argv);
        if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
                return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+       if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+               return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
        return result;
 }

diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 556c506bfa7b5c5ef739d4203c585412a2764073..532b284b10072171c1ca198bd6357780f4579118 100644 (file)
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -46,5 +46,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
        result = run_diff_index(&rev, cached);
        if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
                return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+       if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+               return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
        return result;
 }

diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 2e13716eec985e7274d6f44646c94d8224964b7c..9ab90cb4c535a1328806c05515ba089e1d7147dc 100644 (file)
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -117,23 +117,23 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
                break;
        }
 
-       if (!read_stdin)
-               return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
-                       && DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
+       if (read_stdin) {
+               if (opt->diffopt.detect_rename)
+                       opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
+                                              DIFF_SETUP_USE_CACHE);
+               while (fgets(line, sizeof(line), stdin)) {
+                       unsigned char sha1[20];
 
-       if (opt->diffopt.detect_rename)
-               opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
-                                      DIFF_SETUP_USE_CACHE);
-       while (fgets(line, sizeof(line), stdin)) {
-               unsigned char sha1[20];
-
-               if (get_sha1_hex(line, sha1)) {
-                       fputs(line, stdout);
-                       fflush(stdout);
+                       if (get_sha1_hex(line, sha1)) {
+                               fputs(line, stdout);
+                               fflush(stdout);
+                       }
+                       else
+                               diff_tree_stdin(line);
                }
-               else
-                       diff_tree_stdin(line);
        }
+       if (opt->diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+               return DIFF_OPT_TST(&opt->diffopt, CHECK_FAILED) != 0;
        return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
                && DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
 }

diff --git a/builtin-diff.c b/builtin-diff.c
index 55fb84c730e4141bdb53fec6602a2bdafc99f8fd..9d878f6a71a5a7a53b11112396cb963ba149d6e6 100644 (file)
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -247,7 +247,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
        /* If the user asked for our exit code then don't start a
         * pager or we would end up reporting its exit code instead.
         */
-       if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
+       if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS) &&
+           (!(rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)))
                setup_pager();
 
        /* Do we have --cached and not have a pending object, then
@@ -353,7 +354,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
                                             ent, ents);
        if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
                result = DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
-
+       if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+               return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
        if (1 < rev.diffopt.skip_stat_unmatch)
                refresh_index_quietly();
        return result;

diff --git a/diff.c b/diff.c
index 3dd2f35f7364341bcb5f4507ea36d03925fdc938..823707548422d37e63b1eeae18a25fa63e91b898 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1031,6 +1031,7 @@ struct checkdiff_t {
        const char *filename;
        int lineno, color_diff;
        unsigned ws_rule;
+       unsigned status;
 };
 
 static void checkdiff_consume(void *priv, char *line, unsigned long len)
@@ -1064,6 +1065,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
                        white_space_at_end = 1;
 
                if (space_before_tab || white_space_at_end) {
+                       data->status = 1;
                        printf("%s:%d: %s", data->filename, data->lineno, ws);
                        if (space_before_tab) {
                                printf("space before tab");
@@ -1491,6 +1493,8 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
  free_and_return:
        diff_free_filespec_data(one);
        diff_free_filespec_data(two);
+       if (data.status)
+               DIFF_OPT_SET(o, CHECK_FAILED);
 }
 
 struct diff_filespec *alloc_filespec(const char *path)
@@ -2121,7 +2125,12 @@ int diff_setup_done(struct diff_options *options)
        if (options->output_format & DIFF_FORMAT_NAME_STATUS)
                count++;
        if (options->output_format & DIFF_FORMAT_CHECKDIFF)
+       {
                count++;
+               if (DIFF_OPT_TST(options, QUIET) ||
+                   DIFF_OPT_TST(options, EXIT_WITH_STATUS))
+                       die("--check may not be used with --quiet or --exit-code");
+       }
        if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
                count++;
        if (count > 1)

diff --git a/diff.h b/diff.h
index a52496a1086ccbe8ea90acd6800dd355b1d7ff68..5d50d93a52e7c138b0ecf0ff2b69baa1f9dc5e51 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -59,6 +59,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_ALLOW_EXTERNAL      (1 << 13)
 #define DIFF_OPT_EXIT_WITH_STATUS    (1 << 14)
 #define DIFF_OPT_REVERSE_DIFF        (1 << 15)
+#define DIFF_OPT_CHECK_FAILED        (1 << 16)
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
 #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 6adf9d11d03c3e9b1b7ee1ee46364f0e3a755016..dc538b3e33858623eff8a10d30a488194b58667d 100755 (executable)
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -117,7 +117,6 @@ EOF
 git diff -b > out
 test_expect_success 'another test, with -b' 'git diff expect out'
 
-
 test_expect_success 'check mixed spaces and tabs in indent' '
 
        # This is indented with SP HT SP.
@@ -126,4 +125,140 @@ test_expect_success 'check mixed spaces and tabs in indent' '
 
 '
 
+test_expect_success 'check with no whitespace errors' '
+
+       git commit -m "snapshot" &&
+       echo "foo();" > x &&
+       git diff --check
+
+'
+
+test_expect_failure 'check with trailing whitespace' '
+
+       echo "foo(); " > x &&
+       git diff --check
+
+'
+
+test_expect_failure 'check with space before tab in indent' '
+
+       # indent has space followed by hard tab
+       echo "  foo();" > x &&
+       git diff --check
+
+'
+
+test_expect_failure '--check and --exit-code are exclusive' '
+
+       git checkout x &&
+       git diff --check --exit-code
+
+'
+
+test_expect_failure '--check and --quiet are exclusive' '
+
+       git diff --check --quiet
+
+'
+
+test_expect_success 'check staged with no whitespace errors' '
+
+       echo "foo();" > x &&
+       git add x &&
+       git diff --cached --check
+
+'
+
+test_expect_failure 'check staged with trailing whitespace' '
+
+       echo "foo(); " > x &&
+       git add x &&
+       git diff --cached --check
+
+'
+
+test_expect_failure 'check staged with space before tab in indent' '
+
+       # indent has space followed by hard tab
+       echo "  foo();" > x &&
+       git add x &&
+       git diff --cached --check
+
+'
+
+test_expect_success 'check with no whitespace errors (diff-index)' '
+
+       echo "foo();" > x &&
+       git add x &&
+       git diff-index --check HEAD
+
+'
+
+test_expect_failure 'check with trailing whitespace (diff-index)' '
+
+       echo "foo(); " > x &&
+       git add x &&
+       git diff-index --check HEAD
+
+'
+
+test_expect_failure 'check with space before tab in indent (diff-index)' '
+
+       # indent has space followed by hard tab
+       echo "  foo();" > x &&
+       git add x &&
+       git diff-index --check HEAD
+
+'
+
+test_expect_success 'check staged with no whitespace errors (diff-index)' '
+
+       echo "foo();" > x &&
+       git add x &&
+       git diff-index --cached --check HEAD
+
+'
+
+test_expect_failure 'check staged with trailing whitespace (diff-index)' '
+
+       echo "foo(); " > x &&
+       git add x &&
+       git diff-index --cached --check HEAD
+
+'
+
+test_expect_failure 'check staged with space before tab in indent (diff-index)' '
+
+       # indent has space followed by hard tab
+       echo "  foo();" > x &&
+       git add x &&
+       git diff-index --cached --check HEAD
+
+'
+
+test_expect_success 'check with no whitespace errors (diff-tree)' '
+
+       echo "foo();" > x &&
+       git commit -m "new commit" x &&
+       git diff-tree --check HEAD^ HEAD
+
+'
+
+test_expect_failure 'check with trailing whitespace (diff-tree)' '
+
+       echo "foo(); " > x &&
+       git commit -m "another commit" x &&
+       git diff-tree --check HEAD^ HEAD
+
+'
+
+test_expect_failure 'check with space before tab in indent (diff-tree)' '
+
+       # indent has space followed by hard tab
+       echo "  foo();" > x &&
+       git commit -m "yet another" x &&
+       git diff-tree --check HEAD^ HEAD
+
+'
+
 test_done