{"thread":{"id":"19457","subject":"[PATCH 2/2] diff: Rename QUIET internal option to QUICK","startedAt":"2009-05-23T19:24:48Z","lastAt":"2009-05-25T18:48:09Z","messageCount":5,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"114533","messageId":"1243106690-6385-1-git-send-email-gitster@pobox.com","threadId":"19457","inReplyTo":null,"subject":"[PATCH 0/2] Update \"diff -w --exit-code\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-23T19:24:48Z","receivedAt":"2009-05-23T19:24:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is a re-roll of the earlier patch to change the exit status of \"diff\"\nfamily of commands on whitespace only changes.\n\nEarlier, \"-w\" and friends only affected the displayed of textual diff\noutput and never caused the commands to ignore the presense of\ndifferences.  With this, the commands exit with zero status when the\nchanges are only about whitespaces that you are ignoring.\n\nE.g. starting from a clean slate:\n\n\t$ echo ' a' >a-file ;# one space\n\t$ git add a-file\n\t$ echo '  a' >a-file ;# two spaces\n\t$ git diff --exit-code -w >/dev/null; echo $?\n\nwill give 0, instead of 1.\n\nThe fact that you have changes (i.e. the contents of a-file as a whole has\nchanged) is still reported in the textual part of the output by showing\nthe \"diff --git a/a-file b/b-file\" header and the \"index objname..objname mode\"\nline.  This is not likely to change.\n\nJunio C Hamano (2):\n  diff: change semantics of \"ignore whitespace\" options\n  diff: Rename QUIET internal option to QUICK\n\n builtin-log.c                |    2 +-\n builtin-rev-list.c           |    2 +-\n diff-lib.c                   |    4 +-\n diff.c                       |   39 ++++++++++++++++++++++---\n diff.h                       |    3 +-\n revision.c                   |    2 +-\n t/t4037-whitespace-status.sh |   63 ++++++++++++++++++++++++++++++++++++++++++\n tree-diff.c                  |    3 +-\n 8 files changed, 106 insertions(+), 12 deletions(-)\n create mode 100755 t/t4037-whitespace-status.sh\n"},{"id":"114532","messageId":"1243106690-6385-2-git-send-email-gitster@pobox.com","threadId":"19457","inReplyTo":"1243106690-6385-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/2] diff: change semantics of \"ignore whitespace\" options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-23T19:24:49Z","receivedAt":"2009-05-23T19:24:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Traditionally, the --ignore-whitespace* options have merely meant to tell\nthe diff output routine that some class of differences are not worth\nshowing in the textual diff output, so that the end user has easier time\nto review the remaining (presumably more meaningful) changes.  These\noptions never affected the outcome of the command, given as the exit\nstatus when the --exit-code option was in effect (either directly or\nindirectly).\n\nWhen you have only whitespace changes, however, you might expect\n\n\tgit diff -b --exit-code\n\nto report that there is _no_ change with zero exit status.\n\nChange the semantics of --ignore-whitespace* options to mean more than\n\"omit showing the difference in text\".\n\nThe exit status, when --exit-code is in effect, is computed by checking if\nwe found any differences at the path level, while diff frontends feed\nfilepairs to the diffcore engine.  When \"ignore whitespace\" options are in\neffect, we defer this determination until the very end of diffcore\ntransformation.  We simply do not know until the textual diff is\ngenerated, which comes very late in the pipeline.\n\nWhen --quiet is in effect, various diff frontends optimize by breaking out\nearly from the loop that enumerates the filepairs, when we find the first\npath level difference; when --ignore-whitespace* is used the above change\nautomatically disables this optimization.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c                       |   35 +++++++++++++++++++++--\n diff.h                       |    1 +\n t/t4037-whitespace-status.sh |   63 ++++++++++++++++++++++++++++++++++++++++++\n tree-diff.c                  |    3 +-\n 4 files changed, 98 insertions(+), 4 deletions(-)\n create mode 100755 t/t4037-whitespace-status.sh\n\ndiff --git a/diff.c b/diff.c\nindex f06876b..e4ab696 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2370,6 +2370,21 @@ int diff_setup_done(struct diff_options *options)\n \tif (count > 1)\n \t\tdie(\"--name-only, --name-status, --check and -s are mutually exclusive\");\n \n+\t/*\n+\t * Most of the time we can say \"there are changes\"\n+\t * only by checking if there are changed paths, but\n+\t * --ignore-whitespace* options force us to look\n+\t * inside contets.\n+\t */\n+\n+\tif ((XDF_IGNORE_WHITESPACE|\n+\t     XDF_IGNORE_WHITESPACE_CHANGE|\n+\t     XDF_IGNORE_WHITESPACE_AT_EOL) & options->xdl_opts) {\n+\t\tDIFF_OPT_SET(options, DIFF_FROM_CONTENTS);\n+\t} else {\n+\t\tDIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);\n+\t}\n+\n \tif (DIFF_OPT_TST(options, FIND_COPIES_HARDER))\n \t\toptions->detect_rename = DIFF_DETECT_COPY;\n \n@@ -3322,6 +3337,18 @@ free_queue:\n \tq->nr = q->alloc = 0;\n \tif (options->close_file)\n \t\tfclose(options->file);\n+\n+\t/*\n+\t * Report the contents level differences with HAS_CHANGES;\n+\t * diff_addremove/diff_change does not set the bit when\n+\t * DIFF_FROM_CONTENTS is in effect (e.g. with -w).\n+\t */\n+\tif (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {\n+\t\tif (options->found_changes)\n+\t\t\tDIFF_OPT_SET(options, HAS_CHANGES);\n+\t\telse\n+\t\t\tDIFF_OPT_CLR(options, HAS_CHANGES);\n+\t}\n }\n \n static void diffcore_apply_filter(const char *filter)\n@@ -3458,7 +3485,7 @@ void diffcore_std(struct diff_options *options)\n \tdiff_resolve_rename_copy();\n \tdiffcore_apply_filter(options->filter);\n \n-\tif (diff_queued_diff.nr)\n+\tif (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))\n \t\tDIFF_OPT_SET(options, HAS_CHANGES);\n \telse\n \t\tDIFF_OPT_CLR(options, HAS_CHANGES);\n@@ -3518,7 +3545,8 @@ void diff_addremove(struct diff_options *options,\n \t\tfill_filespec(two, sha1, mode);\n \n \tdiff_queue(&diff_queued_diff, one, two);\n-\tDIFF_OPT_SET(options, HAS_CHANGES);\n+\tif (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))\n+\t\tDIFF_OPT_SET(options, HAS_CHANGES);\n }\n \n void diff_change(struct diff_options *options,\n@@ -3550,7 +3578,8 @@ void diff_change(struct diff_options *options,\n \tfill_filespec(two, new_sha1, new_mode);\n \n \tdiff_queue(&diff_queued_diff, one, two);\n-\tDIFF_OPT_SET(options, HAS_CHANGES);\n+\tif (!DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))\n+\t\tDIFF_OPT_SET(options, HAS_CHANGES);\n }\n \n void diff_unmerge(struct diff_options *options,\ndiff --git a/diff.h b/diff.h\nindex 6616877..538e4f0 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -66,6 +66,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n #define DIFF_OPT_DIRSTAT_CUMULATIVE  (1 << 19)\n #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)\n #define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)\n+#define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)\n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)\n #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)\ndiff --git a/t/t4037-whitespace-status.sh b/t/t4037-whitespace-status.sh\nnew file mode 100755\nindex 0000000..a30b03b\n--- /dev/null\n+++ b/t/t4037-whitespace-status.sh\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+\n+test_description='diff --exit-code with whitespace'\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tmkdir a b &&\n+\techo >c &&\n+\techo >a/d &&\n+\techo >b/e &&\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\techo \" \" >a/d &&\n+\ttest_tick &&\n+\tgit commit -a -m second &&\n+\techo \"  \" >a/d &&\n+\techo \" \" >b/e &&\n+\tgit add a/d\n+'\n+\n+test_expect_success 'diff-tree --exit-code' '\n+\ttest_must_fail git diff --exit-code HEAD^ HEAD &&\n+\ttest_must_fail git diff-tree --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'diff-tree -b --exit-code' '\n+\tgit diff -b --exit-code HEAD^ HEAD &&\n+\tgit diff-tree -b -p --exit-code HEAD^ HEAD &&\n+\tgit diff-tree -b --exit-code HEAD^ HEAD\n+'\n+\n+test_expect_success 'diff-index --cached --exit-code' '\n+\ttest_must_fail git diff --cached --exit-code HEAD &&\n+\ttest_must_fail git diff-index --cached --exit-code HEAD\n+'\n+\n+test_expect_success 'diff-index -b -p --cached --exit-code' '\n+\tgit diff -b --cached --exit-code HEAD &&\n+\tgit diff-index -b -p --cached --exit-code HEAD\n+'\n+\n+test_expect_success 'diff-index --exit-code' '\n+\ttest_must_fail git diff --exit-code HEAD &&\n+\ttest_must_fail git diff-index --exit-code HEAD\n+'\n+\n+test_expect_success 'diff-index -b -p --exit-code' '\n+\tgit diff -b --exit-code HEAD &&\n+\tgit diff-index -b -p --exit-code HEAD\n+'\n+\n+test_expect_success 'diff-files --exit-code' '\n+\ttest_must_fail git diff --exit-code &&\n+\ttest_must_fail git diff-files --exit-code\n+'\n+\n+test_expect_success 'diff-files -b -p --exit-code' '\n+\tgit diff -b --exit-code &&\n+\tgit diff-files -b -p --exit-code\n+'\n+\n+test_done\ndiff --git a/tree-diff.c b/tree-diff.c\nindex edd8394..4b1e3f2 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -280,7 +280,8 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru\n \tint baselen = strlen(base);\n \n \tfor (;;) {\n-\t\tif (DIFF_OPT_TST(opt, QUIET) && DIFF_OPT_TST(opt, HAS_CHANGES))\n+\t\tif (DIFF_OPT_TST(opt, QUIET) &&\n+\t\t    DIFF_OPT_TST(opt, HAS_CHANGES))\n \t\t\tbreak;\n \t\tif (opt->nr_paths) {\n \t\t\tskip_uninteresting(t1, base, baselen, opt);\n-- \n1.6.3.1.145.gb74d77\n"},{"id":"114531","messageId":"1243106690-6385-3-git-send-email-gitster@pobox.com","threadId":"19457","inReplyTo":"1243106690-6385-2-git-send-email-gitster@pobox.com","subject":"[PATCH 2/2] diff: Rename QUIET internal option to QUICK","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-23T19:24:50Z","receivedAt":"2009-05-23T19:24:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The option \"QUIET\" primarily meant \"find if we have _any_ difference as\nquick as possible and report\", which means we often do not even have to\nlook at blobs if we know the trees are different by looking at the higher\nlevel (e.g. \"diff-tree A B\").  As a side effect, because there is no point\nshowing one change that we happened to have found first, it also enables\nNO_OUTPUT and EXIT_WITH_STATUS options, making the end result look quiet.\n\nRename the internal option to QUICK to reflect this better; it also makes\ngrepping the source tree much easier, as there are other kinds of QUIET\noption everywhere.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-log.c      |    2 +-\n builtin-rev-list.c |    2 +-\n diff-lib.c         |    4 ++--\n diff.c             |    4 ++--\n diff.h             |    2 +-\n revision.c         |    2 +-\n tree-diff.c        |    2 +-\n 7 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5eaec5d..80624f5 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -536,7 +536,7 @@ static int reopen_stdout(struct commit *commit, struct rev_info *rev)\n \n \tget_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);\n \n-\tif (!DIFF_OPT_TST(&rev->diffopt, QUIET))\n+\tif (!DIFF_OPT_TST(&rev->diffopt, QUICK))\n \t\tfprintf(realstdout, \"%s\\n\", filename.buf + outdir_offset);\n \n \tif (freopen(filename.buf, \"w\", stdout) == NULL)\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 38a8f23..61d3126 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -315,7 +315,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tmemset(&info, 0, sizeof(info));\n \tinfo.revs = &revs;\n \n-\tquiet = DIFF_OPT_TST(&revs.diffopt, QUIET);\n+\tquiet = DIFF_OPT_TST(&revs.diffopt, QUICK);\n \tfor (i = 1 ; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \ndiff --git a/diff-lib.c b/diff-lib.c\nindex a310fb2..a549ee6 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -73,7 +73,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tint changed;\n \n-\t\tif (DIFF_OPT_TST(&revs->diffopt, QUIET) &&\n+\t\tif (DIFF_OPT_TST(&revs->diffopt, QUICK) &&\n \t\t\tDIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))\n \t\t\tbreak;\n \n@@ -520,7 +520,7 @@ int index_differs_from(const char *def, int diff_flags)\n \n \tinit_revisions(&rev, NULL);\n \tsetup_revisions(0, NULL, &rev, def);\n-\tDIFF_OPT_SET(&rev.diffopt, QUIET);\n+\tDIFF_OPT_SET(&rev.diffopt, QUICK);\n \tDIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n \trev.diffopt.flags |= diff_flags;\n \trun_diff_index(&rev, 1);\ndiff --git a/diff.c b/diff.c\nindex e4ab696..eb3a713 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2445,7 +2445,7 @@ int diff_setup_done(struct diff_options *options)\n \t * to have found.  It does not make sense not to return with\n \t * exit code in such a case either.\n \t */\n-\tif (DIFF_OPT_TST(options, QUIET)) {\n+\tif (DIFF_OPT_TST(options, QUICK)) {\n \t\toptions->output_format = DIFF_FORMAT_NO_OUTPUT;\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \t}\n@@ -2636,7 +2636,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \telse if (!strcmp(arg, \"--quiet\"))\n-\t\tDIFF_OPT_SET(options, QUIET);\n+\t\tDIFF_OPT_SET(options, QUICK);\n \telse if (!strcmp(arg, \"--ext-diff\"))\n \t\tDIFF_OPT_SET(options, ALLOW_EXTERNAL);\n \telse if (!strcmp(arg, \"--no-ext-diff\"))\ndiff --git a/diff.h b/diff.h\nindex 538e4f0..a7e7ccb 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -55,7 +55,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n #define DIFF_OPT_COLOR_DIFF          (1 <<  8)\n #define DIFF_OPT_COLOR_DIFF_WORDS    (1 <<  9)\n #define DIFF_OPT_HAS_CHANGES         (1 << 10)\n-#define DIFF_OPT_QUIET               (1 << 11)\n+#define DIFF_OPT_QUICK               (1 << 11)\n #define DIFF_OPT_NO_INDEX            (1 << 12)\n #define DIFF_OPT_ALLOW_EXTERNAL      (1 << 13)\n #define DIFF_OPT_EXIT_WITH_STATUS    (1 << 14)\ndiff --git a/revision.c b/revision.c\nindex 18b7ebb..1c114ab 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -800,7 +800,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)\n \trevs->ignore_merges = 1;\n \trevs->simplify_history = 1;\n \tDIFF_OPT_SET(&revs->pruning, RECURSIVE);\n-\tDIFF_OPT_SET(&revs->pruning, QUIET);\n+\tDIFF_OPT_SET(&revs->pruning, QUICK);\n \trevs->pruning.add_remove = file_add_remove;\n \trevs->pruning.change = file_change;\n \trevs->lifo = 1;\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 4b1e3f2..6dec88b 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -280,7 +280,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru\n \tint baselen = strlen(base);\n \n \tfor (;;) {\n-\t\tif (DIFF_OPT_TST(opt, QUIET) &&\n+\t\tif (DIFF_OPT_TST(opt, QUICK) &&\n \t\t    DIFF_OPT_TST(opt, HAS_CHANGES))\n \t\t\tbreak;\n \t\tif (opt->nr_paths) {\n-- \n1.6.3.1.145.gb74d77\n"},{"id":"114657","messageId":"20090525110429.GB27070@coredump.intra.peff.net","threadId":"19457","inReplyTo":"1243106690-6385-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/2] diff: change semantics of \"ignore whitespace\" options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-05-25T11:04:29Z","receivedAt":"2009-05-25T11:04:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 23, 2009 at 12:24:49PM -0700, Junio C Hamano wrote:\n\n> When --quiet is in effect, various diff frontends optimize by breaking out\n> early from the loop that enumerates the filepairs, when we find the first\n> path level difference; when --ignore-whitespace* is used the above change\n> automatically disables this optimization.\n\nBoth patches in this series look good to me, with one slight exception:\n\n> +\tif ((XDF_IGNORE_WHITESPACE|\n> +\t     XDF_IGNORE_WHITESPACE_CHANGE|\n> +\t     XDF_IGNORE_WHITESPACE_AT_EOL) & options->xdl_opts) {\n> +\t\tDIFF_OPT_SET(options, DIFF_FROM_CONTENTS);\n> +\t} else {\n> +\t\tDIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);\n> +\t}\n\nWe have DIFF_XDL_TST these days; while it is currently equivalent to\nwhat you do here, I thought part of the purpose was to abstract it to\nmake it simpler to later add master/slave magic.\n\n-Peff\n"},{"id":"114684","messageId":"7vhbz9vx1y.fsf@alter.siamese.dyndns.org","threadId":"19457","inReplyTo":"20090525110429.GB27070@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] diff: change semantics of \"ignore whitespace\" options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-25T18:48:09Z","receivedAt":"2009-05-25T18:48:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> +\tif ((XDF_IGNORE_WHITESPACE|\n>> +\t     XDF_IGNORE_WHITESPACE_CHANGE|\n>> +\t     XDF_IGNORE_WHITESPACE_AT_EOL) & options->xdl_opts) {\n>> +\t\tDIFF_OPT_SET(options, DIFF_FROM_CONTENTS);\n>> +\t} else {\n>> +\t\tDIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);\n>> +\t}\n>\n> We have DIFF_XDL_TST these days;...\n\nHeh, nobody seems to use it though.\n\nNow the part reads\n\n\tif (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||\n\t    DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||\n\t    DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))\n\t\tDIFF_OPT_SET(options, DIFF_FROM_CONTENTS);\n\telse\n\t\tDIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);\n\nThanks.\n"}]}