diff --check: minor fixups

There is no reason --exit-code and --check-diff must be mutually
exclusive, so assign different bits to different results and allow them
to be returned from the command.  Introduce diff_result_code() to factor
out the common code to decide final status code based on diffopt
settings and use it everywhere.

Update tests to match the above fix.

Turning pager off when "diff --check" is used is a regression.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 4afc8724e71449667ce22ac8efd0910b7a57ef42..9c0411165691bb1c3a581e9927e124e55fbd7bfd 100644 (file)
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -31,9 +31,5 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
        if (!rev.diffopt.output_format)
                rev.diffopt.output_format = DIFF_FORMAT_RAW;
        result = run_diff_files_cmd(&rev, argc, argv);
        if (!rev.diffopt.output_format)
                rev.diffopt.output_format = DIFF_FORMAT_RAW;
        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;
+       return diff_result_code(&rev.diffopt, result);

 }
 }

diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 532b284b10072171c1ca198bd6357780f4579118..0f2390a20a6a138388564cd5e0395aee0a48b59c 100644 (file)
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -44,9 +44,5 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
                return -1;
        }
        result = run_diff_index(&rev, cached);
                return -1;
        }
        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;
+       return diff_result_code(&rev.diffopt, result);

 }
 }

diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 9ab90cb4c535a1328806c05515ba089e1d7147dc..ebc50efbd22f7fa294d66d73d7b2c3084931b4b7 100644 (file)
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -132,8 +132,6 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
                                diff_tree_stdin(line);
                }
        }
                                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);
+
+       return diff_result_code(&opt->diffopt, 0);

 }
 }

diff --git a/builtin-diff.c b/builtin-diff.c
index 9d878f6a71a5a7a53b11112396cb963ba149d6e6..29365a0b17348982ea806add83f3c95bea43d4f6 100644 (file)
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -244,11 +244,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
        DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
        DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 
        DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
        DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 

-       /* If the user asked for our exit code then don't start a
+       /*
+        * If the user asked for our exit code then don't start a

         * pager or we would end up reporting its exit code instead.
         */
         * pager or we would end up reporting its exit code instead.
         */

-       if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS) &&
-           (!(rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)))
+       if (!DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))

                setup_pager();
 
        /* Do we have --cached and not have a pending object, then
                setup_pager();
 
        /* Do we have --cached and not have a pending object, then


@@ -352,10 +352,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
        else
                result = builtin_diff_combined(&rev, argc, argv,
                                             ent, ents);
        else
                result = builtin_diff_combined(&rev, argc, argv,
                                             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;
+       result = diff_result_code(&rev.diffopt, result);

        if (1 < rev.diffopt.skip_stat_unmatch)
                refresh_index_quietly();
        return result;
        if (1 < rev.diffopt.skip_stat_unmatch)
                refresh_index_quietly();
        return result;

diff --git a/diff.c b/diff.c
index 823707548422d37e63b1eeae18a25fa63e91b898..3e46ff8a75c5840a5f3374aa76e93cfb468c86f1 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -2125,12 +2125,7 @@ int diff_setup_done(struct diff_options *options)
        if (options->output_format & DIFF_FORMAT_NAME_STATUS)
                count++;
        if (options->output_format & DIFF_FORMAT_CHECKDIFF)
        if (options->output_format & DIFF_FORMAT_NAME_STATUS)
                count++;
        if (options->output_format & DIFF_FORMAT_CHECKDIFF)

-       {

                count++;
                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)
        if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
                count++;
        if (count > 1)


@@ -3180,6 +3175,20 @@ void diffcore_std(struct diff_options *options)
                DIFF_OPT_CLR(options, HAS_CHANGES);
 }
 
                DIFF_OPT_CLR(options, HAS_CHANGES);
 }
 

+int diff_result_code(struct diff_options *opt, int status)
+{
+       int result = 0;
+       if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
+           !(opt->output_format & DIFF_FORMAT_CHECKDIFF))
+               return status;
+       if (DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
+           DIFF_OPT_TST(opt, HAS_CHANGES))
+               result |= 01;
+       if ((opt->output_format & DIFF_FORMAT_CHECKDIFF) &&
+           DIFF_OPT_TST(opt, CHECK_FAILED))
+               result |= 02;
+       return result;
+}

 
 void diff_addremove(struct diff_options *options,
                    int addremove, unsigned mode,
 
 void diff_addremove(struct diff_options *options,
                    int addremove, unsigned mode,

diff --git a/diff.h b/diff.h
index 5d50d93a52e7c138b0ecf0ff2b69baa1f9dc5e51..7e8000a5d7ed7b36669abaca38111c43038092aa 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -247,4 +247,6 @@ extern int run_diff_index(struct rev_info *revs, int cached);
 extern int do_diff_cache(const unsigned char *, struct diff_options *);
 extern int diff_flush_patch_id(struct diff_options *, unsigned char *);
 
 extern int do_diff_cache(const unsigned char *, struct diff_options *);
 extern int diff_flush_patch_id(struct diff_options *, unsigned char *);
 

+extern int diff_result_code(struct diff_options *, int);
+

 #endif /* DIFF_H */
 #endif /* DIFF_H */

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index dc538b3e33858623eff8a10d30a488194b58667d..757a27abdbbffe27637dc68126d59fc1ea128119 100755 (executable)
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -148,14 +148,14 @@ test_expect_failure 'check with space before tab in indent' '
 
 '
 
 
 '
 

-test_expect_failure '--check and --exit-code are exclusive' '
+test_expect_success '--check and --exit-code are not exclusive' '

 
        git checkout x &&
        git diff --check --exit-code
 
 '
 
 
        git checkout x &&
        git diff --check --exit-code
 
 '
 

-test_expect_failure '--check and --quiet are exclusive' '
+test_expect_success '--check and --quiet are not exclusive' '

 
        git diff --check --quiet
 
 
        git diff --check --quiet