{"thread":{"id":"19439","subject":"git-diff: must --exit-code work with --ignore* options?","startedAt":"2009-05-22T14:01:57Z","lastAt":"2009-09-08T20:58:38Z","messageCount":9,"participants":["Jim Meyering","Junio C Hamano","Thell Fowler"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"114467","messageId":"87k549dyne.fsf@meyering.net","threadId":"19439","inReplyTo":null,"subject":"git-diff: must --exit-code work with --ignore* options?","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2009-05-22T14:01:57Z","receivedAt":"2009-05-22T14:01:57Z","isPatch":false,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"git-diff's --quiet option works how I'd expect with --ignore-space-at-eol\nas long as I'm also using --no-index:\n\n    $ echo>b; echo \\ >c; git diff --no-index --quiet --ignore-space-at-eol b c \\\n      && echo good\n    good\n\nBut in what I think of as normal operation (i.e., without --no-index),\n--exit-code (or --quiet) makes git-diff say there were differences,\neven when they have been ignored:\n\n    # do this in an empty directory\n    $ git init -q; echo>k; git add .; git commit -q -m. .; echo \\ >k\n    $ git diff --ignore-space-at-eol --quiet || echo bad\n    bad\n\nSame problem with --ignore-space-change.\n\n-------------------\nIn the surprising case, builtin-diff.c's builtin_diff_files calls\ndiff_result_code, which returns nonzero due to this:\n\n          if (diff_queued_diff.nr)\n                  DIFF_OPT_SET(options, HAS_CHANGES);\n          else\n                  DIFF_OPT_CLR(options, HAS_CHANGES);\n\nHowever, the queued diffs may contain only ignorable changes.\n\nWith --no-index, it takes a different code path and uses\ndiffopt.found_changes to produce the desired exit status.\n"},{"id":"114475","messageId":"7vvdnt869j.fsf@alter.siamese.dyndns.org","threadId":"19439","inReplyTo":"87k549dyne.fsf@meyering.net","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-22T16:14:00Z","receivedAt":"2009-05-22T16:14:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> git-diff's --quiet option works how I'd expect with --ignore-space-at-eol\n> as long as I'm also using --no-index:\n>\n>     $ echo>b; echo \\ >c; git diff --no-index --quiet --ignore-space-at-eol b c \\\n>       && echo good\n>     good\n>\n> But in what I think of as normal operation (i.e., without --no-index),\n> --exit-code (or --quiet) makes git-diff say there were differences,\n> even when they have been ignored:\n>\n>     # do this in an empty directory\n>     $ git init -q; echo>k; git add .; git commit -q -m. .; echo \\ >k\n>     $ git diff --ignore-space-at-eol --quiet || echo bad\n>     bad\n\nI am slightly torn about this, in that I can picture myself saying that\nthis is unintuitive on some different days, but not today ;-)\n\nIf you look at the output (i.e. no --quiet), you would see that the blob\nchanges are still reported for the path.  E.g.  you would see something\nlike...\n\n\t$ git diff --ignore-space-at-eol\n        diff --git a/k b/k\n        index 8b13789..8d1c8b6 100644\n\nThe \"index\" line is still showing that there _is_ a difference.\n\nThe --ignore-* options are there merely to tell git what changes are not\nworth _showing_ in the textual part of the patch, in order to cut down the\namount of the output.  It never affects the outcome.\n\nSo if anything, I think --no-index codepath is what's buggy; if it does\nnot report the blob difference that is a different matter, though.\n"},{"id":"114482","messageId":"87eiuhdnw9.fsf@meyering.net","threadId":"19439","inReplyTo":"7vvdnt869j.fsf@alter.siamese.dyndns.org","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2009-05-22T17:54:14Z","receivedAt":"2009-05-22T17:54:14Z","isPatch":false,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano wrote:\n> Jim Meyering <jim@meyering.net> writes:\n>> git-diff's --quiet option works how I'd expect with --ignore-space-at-eol\n>> as long as I'm also using --no-index:\n>>\n>>     $ echo>b; echo \\ >c; git diff --no-index --quiet --ignore-space-at-eol b c \\\n>>       && echo good\n>>     good\n>>\n>> But in what I think of as normal operation (i.e., without --no-index),\n>> --exit-code (or --quiet) makes git-diff say there were differences,\n>> even when they have been ignored:\n>>\n>>     # do this in an empty directory\n>>     $ git init -q; echo>k; git add .; git commit -q -m. .; echo \\ >k\n>>     $ git diff --ignore-space-at-eol --quiet || echo bad\n>>     bad\n>\n> I am slightly torn about this, in that I can picture myself saying that\n> this is unintuitive on some different days, but not today ;-)\n\nThanks for the quick reply.  Here's why I noticed:\n\nI wanted to ensure that the only changes induced by commit C were\nto trailing blanks.  I wrote something like this, expecting to be able\nto deal with the exception:\n\n    git --quiet --ignore-space-at-eol --quiet C^..C || handle_unexpected\n\nBut handle_unexpected was always being invoked.\nI was surprised because GNU diff's --ignore-all-space (-w) option does\nwork the way I expected:\n\n    $ echo>b; echo \\ >c; diff -w b c && echo $?\n    0\n\n> If you look at the output (i.e. no --quiet), you would see that the blob\n> changes are still reported for the path.  E.g.  you would see something\n> like...\n>\n> \t$ git diff --ignore-space-at-eol\n>         diff --git a/k b/k\n>         index 8b13789..8d1c8b6 100644\n>\n> The \"index\" line is still showing that there _is_ a difference.\n\nI did see that, to my chagrin:\nif using a --ignore-... option had also suppressed those, I could\nhave tested for empty output instead of exit status.\n\n> The --ignore-* options are there merely to tell git what changes are not\n> worth _showing_ in the textual part of the patch, in order to cut down the\n> amount of the output.  It never affects the outcome.\n>\n> So if anything, I think --no-index codepath is what's buggy; if it does\n> not report the blob difference that is a different matter, though.\n\nIf need be, I can work around it.\n"},{"id":"114487","messageId":"7v7i087twu.fsf@alter.siamese.dyndns.org","threadId":"19439","inReplyTo":"87eiuhdnw9.fsf@meyering.net","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-22T20:40:49Z","receivedAt":"2009-05-22T20:40:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> Junio C Hamano wrote:\n>> Jim Meyering <jim@meyering.net> writes:\n>>>\n>>>     # do this in an empty directory\n>>>     $ git init -q; echo>k; git add .; git commit -q -m. .; echo \\ >k\n>>>     $ git diff --ignore-space-at-eol --quiet || echo bad\n>>>     bad\n>>\n>> I am slightly torn about this, in that I can picture myself saying that\n>> this is unintuitive on some different days, but not today ;-)\n>\n> Thanks for the quick reply.  Here's why I noticed:\n> ...\n\nIt seems that today is already \"some different day\" ;-) We could do\nsomething like this patch.\n\nWhile in the longer term I think it may make the world a better place by\nbeing more consistent with what users expect, I am not sure at what\nrevision boundary we should introduce such a semantic change.\n\nWe could always declare this a bug and apply the \"fix\" at any time.  It's\nall perception ;-).\n\n-- >8 --\nSubject: [PATCH] diff --quiet: special case \"ignore whitespace\" options\n\nThe 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\nTraditionally, 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\nThese two classes of options are incompatible.  When you have only\nwhitespace changes, you would expect:\n\n\tgit diff -b --quiet\n\nto report that there is _no_ change.  This is unfortunately not the case,\nhowever, if there are differences to be reported if the command was run\nwithout --quiet; there _is_ a change, and the command still exits with\nnon-zero status.\n\nAnd that is wrong.\n\nChange the semantics of --ignore-whitespace* options to mean more than\n\"omit showing the difference in text\".  When these options are used, the\ninternal \"quick\" optimization is turned off, and the status reported with\nthe --exit-code option will now match if any the textual diff output is\nactually produced.\n\nAlso rename the internal option \"QUIET\" to \"QUICK\" to better reflect what\nits true purpose is.\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             |   39 ++++++++++++++++++++++++++++++++++++---\n diff.h             |    3 ++-\n revision.c         |    2 +-\n tree-diff.c        |    2 +-\n 7 files changed, 44 insertions(+), 10 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 f06876b..f2ed2ac 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@@ -2430,9 +2445,19 @@ 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+\n+\t\t/*\n+\t\t * QUICK means \"if we find any difference return early\n+\t\t * and say 'there is a difference'\", and we often do\n+\t\t * not even look at the blobs.  Some options would not\n+\t\t * be compatible with this optimization, so we turn it\n+\t\t * off, make it into \"no output but exit with status\".\n+\t\t */\n+\t\tif (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))\n+\t\t\tDIFF_OPT_CLR(options, QUICK);\n \t}\n \n \treturn 0;\n@@ -2621,7 +2646,8 @@ 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\t/* see postprocessing in diff_setup_done() */\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\"))\n@@ -3322,6 +3348,13 @@ free_queue:\n \tq->nr = q->alloc = 0;\n \tif (options->close_file)\n \t\tfclose(options->file);\n+\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 +3491,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);\ndiff --git a/diff.h b/diff.h\nindex 6616877..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)\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/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 edd8394..ac85a55 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) && DIFF_OPT_TST(opt, HAS_CHANGES))\n+\t\tif (DIFF_OPT_TST(opt, QUICK) && 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.70.ga80aa\n"},{"id":"114493","messageId":"878wkoe0v2.fsf@meyering.net","threadId":"19439","inReplyTo":"7v7i087twu.fsf@alter.siamese.dyndns.org","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2009-05-23T07:26:25Z","receivedAt":"2009-05-23T07:26:25Z","isPatch":false,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano wrote:\n> Jim Meyering <jim@meyering.net> writes:\n>> Junio C Hamano wrote:\n>>> Jim Meyering <jim@meyering.net> writes:\n>>>>\n>>>>     # do this in an empty directory\n>>>>     $ git init -q; echo>k; git add .; git commit -q -m. .; echo \\ >k\n>>>>     $ git diff --ignore-space-at-eol --quiet || echo bad\n>>>>     bad\n>>>\n>>> I am slightly torn about this, in that I can picture myself saying that\n>>> this is unintuitive on some different days, but not today ;-)\n>>\n>> Thanks for the quick reply.  Here's why I noticed:\n>> ...\n>\n> It seems that today is already \"some different day\" ;-) We could do\n> something like this patch.\n>\n> While in the longer term I think it may make the world a better place by\n> being more consistent with what users expect, I am not sure at what\n> revision boundary we should introduce such a semantic change.\n>\n> -- >8 --\n> Subject: [PATCH] diff --quiet: special case \"ignore whitespace\" options\n> ...\n\nWow.  And now a patch.  Service with style ;-)\n\n> We could always declare this a bug and apply the \"fix\" at any time.  It's\n> all perception ;-).\n\nThe declare-it-a-bug option sounds sensible, since I doubt anyone\neven noticed, much less relied on, the changing behavior.\n\nThank you!\n"},{"id":"122119","messageId":"87skf9uv3r.fsf@meyering.net","threadId":"19439","inReplyTo":"7v7i087twu.fsf@alter.siamese.dyndns.org","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2009-08-30T16:25:44Z","receivedAt":"2009-08-30T16:25:44Z","isPatch":false,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano wrote:\n> Jim Meyering <jim@meyering.net> writes:\n>> Junio C Hamano wrote:\n>>> Jim Meyering <jim@meyering.net> writes:\n>>>>\n>>>>     # do this in an empty directory\n>>>>     $ git init -q; echo>k; git add .; git commit -q -m. .; echo \\ >k\n>>>>     $ git diff --ignore-space-at-eol --quiet || echo bad\n>>>>     bad\n>>>\n>>> I am slightly torn about this, in that I can picture myself saying that\n>>> this is unintuitive on some different days, but not today ;-)\n>>\n>> Thanks for the quick reply.  Here's why I noticed:\n>> ...\n>\n> It seems that today is already \"some different day\" ;-) We could do\n> something like this patch.\n>\n> While in the longer term I think it may make the world a better place by\n> being more consistent with what users expect, I am not sure at what\n> revision boundary we should introduce such a semantic change.\n>\n> We could always declare this a bug and apply the \"fix\" at any time.  It's\n> all perception ;-).\n>\n> -- >8 --\n> Subject: [PATCH] diff --quiet: special case \"ignore whitespace\" options\n>\n> The option \"QUIET\" primarily meant \"find if we have _any_ difference as\n> quick as possible and report\", which means we often do not even have to\n> look at blobs if we know the trees are different by looking at the higher\n> level (e.g. \"diff-tree A B\").  As a side effect, because there is no point\n> showing one change that we happened to have found first, it also enables\n> NO_OUTPUT and EXIT_WITH_STATUS options, making the end result look quiet.\n>\n> Traditionally, the --ignore-whitespace* options have merely meant to tell\n> the diff output routine that some class of differences are not worth\n> showing in the textual diff output, so that the end user has easier time\n> to review the remaining (presumably more meaningful) changes.  These\n> options never affected the outcome of the command, given as the exit\n> status when the --exit-code option was in effect (either directly or\n> indirectly).\n>\n> These two classes of options are incompatible.  When you have only\n> whitespace changes, you would expect:\n>\n> \tgit diff -b --quiet\n>\n> to report that there is _no_ change.  This is unfortunately not the case,\n> however, if there are differences to be reported if the command was run\n> without --quiet; there _is_ a change, and the command still exits with\n> non-zero status.\n>\n> And that is wrong.\n>\n> Change the semantics of --ignore-whitespace* options to mean more than\n> \"omit showing the difference in text\".  When these options are used, the\n> internal \"quick\" optimization is turned off, and the status reported with\n> the --exit-code option will now match if any the textual diff output is\n> actually produced.\n>\n> Also rename the internal option \"QUIET\" to \"QUICK\" to better reflect what\n> its true purpose is.\n\nThanks again.\nIf there's anything I can to do help (add a test?), let me know.\n"},{"id":"122129","messageId":"7vljl1dpud.fsf@alter.siamese.dyndns.org","threadId":"19439","inReplyTo":"87skf9uv3r.fsf@meyering.net","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-30T20:11:22Z","receivedAt":"2009-08-30T20:11:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> Junio C Hamano wrote:\n> ...\n>> Subject: [PATCH] diff --quiet: special case \"ignore whitespace\" options\n>> ...\n>> Change the semantics of --ignore-whitespace* options to mean more than\n>> \"omit showing the difference in text\".  When these options are used, the\n>> internal \"quick\" optimization is turned off, and the status reported with\n>> the --exit-code option will now match if any the textual diff output is\n>> actually produced.\n>>\n>> Also rename the internal option \"QUIET\" to \"QUICK\" to better reflect what\n>> its true purpose is.\n>\n> Thanks again.\n> If there's anything I can to do help (add a test?), let me know.\n\nThe change has been cooking in 'next' and hopefully be in 1.7.0.  I think\nthe updated series adds its own test script, too.\n\nUsing it in every day scenario, and reporting any breakage you notice\nbefore 1.7.0 happens, would be greatly appreciated.\n\nThanks.\n"},{"id":"122134","messageId":"87fxb9ujxl.fsf@meyering.net","threadId":"19439","inReplyTo":"7vljl1dpud.fsf@alter.siamese.dyndns.org","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2009-08-30T20:27:02Z","receivedAt":"2009-08-30T20:27:02Z","isPatch":false,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"Junio C Hamano wrote:\n> Jim Meyering <jim@meyering.net> writes:\n>> Junio C Hamano wrote:\n>> ...\n>>> Subject: [PATCH] diff --quiet: special case \"ignore whitespace\" options\n>>> ...\n>>> Change the semantics of --ignore-whitespace* options to mean more than\n>>> \"omit showing the difference in text\".  When these options are used, the\n>>> internal \"quick\" optimization is turned off, and the status reported with\n>>> the --exit-code option will now match if any the textual diff output is\n>>> actually produced.\n>>>\n>>> Also rename the internal option \"QUIET\" to \"QUICK\" to better reflect what\n>>> its true purpose is.\n>>\n>> Thanks again.\n>> If there's anything I can to do help (add a test?), let me know.\n>\n> The change has been cooking in 'next' and hopefully be in 1.7.0.  I think\n> the updated series adds its own test script, too.\n>\n> Using it in every day scenario, and reporting any breakage you notice\n> before 1.7.0 happens, would be greatly appreciated.\n\nOh!  I am using next (will test!), and even searched log summary output,\nbut obviously my search was too cursory or just inaccurate.\n\nI glanced through it and it looks fine (of course!).\nI spotted one typo, and suggest a second change that's barely worth\nmentioning, both in comments:\n\ndiff --git a/diff.c b/diff.c\nindex 91d6ea2..24bd3fc 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2382,7 +2382,7 @@ int diff_setup_done(struct diff_options *options)\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 * inside contents.\n \t */\n\n \tif (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||\n@@ -3346,7 +3346,7 @@ free_queue:\n \t\tfclose(options->file);\n\n \t/*\n-\t * Report the contents level differences with HAS_CHANGES;\n+\t * Report the content-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"},{"id":"122729","messageId":"alpine.WNT.2.00.0909081457190.3732@GWNotebook","threadId":"19439","inReplyTo":"7vljl1dpud.fsf@alter.siamese.dyndns.org","subject":"Re: git-diff: must --exit-code work with --ignore* options?","fromName":"Thell Fowler","fromEmail":"git@tbfowler.name","sentAt":"2009-09-08T20:58:38Z","receivedAt":"2009-09-08T20:58:38Z","isPatch":false,"sender":{"key":"git@tbfowler.name","avatar":"https://gravatar.com/avatar/8b82510e07f3a8a77c8f41ce04c75c3b59b7e9c2d6f495b8f2ed834359efa6de?d=mp&s=160"},"body":"On Sun, 30 Aug 2009, Junio C Hamano wrote:\n\n> Jim Meyering <jim@meyering.net> writes:\n>\n>> Junio C Hamano wrote:\n>> ...\n>>> Subject: [PATCH] diff --quiet: special case \"ignore whitespace\" options\n>>> ...\n>>> Change the semantics of --ignore-whitespace* options to mean more than\n>>> \"omit showing the difference in text\".  When these options are used, the\n>>> internal \"quick\" optimization is turned off, and the status reported with\n>>> the --exit-code option will now match if any the textual diff output is\n>>> actually produced.\n>>>\n>>> Also rename the internal option \"QUIET\" to \"QUICK\" to better reflect what\n>>> its true purpose is.\n>>\n>> Thanks again.\n>> If there's anything I can to do help (add a test?), let me know.\n>\n> The change has been cooking in 'next' and hopefully be in 1.7.0.  I think\n> the updated series adds its own test script, too.\n>\n> Using it in every day scenario, and reporting any breakage you notice\n> before 1.7.0 happens, would be greatly appreciated.\n>\n> Thanks.\n\nPerhaps I'm expected something different than what I _should_ be \nexpecting, but shouldn't --quiet always return the same as --exit-code?\n\n# Cut/Paste example\nmkdir test_ws_quiet && cd test_ws_quiet && git init\nprintf \"foo bar  \\n\\n\" >f1.txt\ngit add .\ngit commit -m 'f text'\nprintf \"foo  bar\\n\\n\" >f1.txt\ngit commit -a -m 'f with diff white-space in middle & end'\ngit diff -w --exit-code HEAD^ >/dev/null\necho $?\n# returns '0' which it should\ngit diff -w --quiet HEAD^\necho $?\n# returns '0' which it should\ngit diff -b --exit-code HEAD^ >/dev/null\necho $?\n# returns '0' which it should\ngit diff -b --quiet HEAD^ >/dev/null\necho $?\n# returns '0' which it should\ngit diff --ignore-space-at-eol --exit-code HEAD^ >/dev/null\necho $?\n# returns '1' which it should\ngit diff --ignore-space-at-eol --quiet HEAD^\necho $?\n#returns '0' <=== Unexpected.\n\n#\n# Next phase\n#\nprintf \"foobar\\n\\n\">f1.txt\ngit commit -a -m 'f without any spaces'\ngit diff -w --exit-code HEAD^ >/dev/null\necho $?\n# returns '0' which it should\ngit diff -w --quiet HEAD^\necho $?\n# returns '0' which it should\ngit diff -b --exit-code HEAD^ >/dev/null\necho $?\n# returns '1' which it should\ngit diff -b --quiet HEAD^ >/dev/null\necho $?\n# returns '0' <=== Unexpected\ngit diff --ignore-space-at-eol --exit-code HEAD^ >/dev/null\necho $?\n# returns '1' which it should\ngit diff --ignore-space-at-eol --quiet HEAD^\necho $?\n#returns '0' <=== Unexpected.\n\n-- \nThell\n"}]}