* git-diff: must --exit-code work with --ignore* options?
@ 2009-05-22 14:01 Jim Meyering
2009-05-22 16:14 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Jim Meyering @ 2009-05-22 14:01 UTC (permalink / raw)
To: git list
git-diff's --quiet option works how I'd expect with --ignore-space-at-eol
as long as I'm also using --no-index:
$ echo>b; echo \ >c; git diff --no-index --quiet --ignore-space-at-eol b c \
&& echo good
good
But in what I think of as normal operation (i.e., without --no-index),
--exit-code (or --quiet) makes git-diff say there were differences,
even when they have been ignored:
# do this in an empty directory
$ git init -q; echo>k; git add .; git commit -q -m. .; echo \ >k
$ git diff --ignore-space-at-eol --quiet || echo bad
bad
Same problem with --ignore-space-change.
-------------------
In the surprising case, builtin-diff.c's builtin_diff_files calls
diff_result_code, which returns nonzero due to this:
if (diff_queued_diff.nr)
DIFF_OPT_SET(options, HAS_CHANGES);
else
DIFF_OPT_CLR(options, HAS_CHANGES);
However, the queued diffs may contain only ignorable changes.
With --no-index, it takes a different code path and uses
diffopt.found_changes to produce the desired exit status.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-05-22 14:01 git-diff: must --exit-code work with --ignore* options? Jim Meyering
@ 2009-05-22 16:14 ` Junio C Hamano
2009-05-22 17:54 ` Jim Meyering
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2009-05-22 16:14 UTC (permalink / raw)
To: Jim Meyering; +Cc: git list
Jim Meyering <jim@meyering.net> writes:
> git-diff's --quiet option works how I'd expect with --ignore-space-at-eol
> as long as I'm also using --no-index:
>
> $ echo>b; echo \ >c; git diff --no-index --quiet --ignore-space-at-eol b c \
> && echo good
> good
>
> But in what I think of as normal operation (i.e., without --no-index),
> --exit-code (or --quiet) makes git-diff say there were differences,
> even when they have been ignored:
>
> # do this in an empty directory
> $ git init -q; echo>k; git add .; git commit -q -m. .; echo \ >k
> $ git diff --ignore-space-at-eol --quiet || echo bad
> bad
I am slightly torn about this, in that I can picture myself saying that
this is unintuitive on some different days, but not today ;-)
If you look at the output (i.e. no --quiet), you would see that the blob
changes are still reported for the path. E.g. you would see something
like...
$ git diff --ignore-space-at-eol
diff --git a/k b/k
index 8b13789..8d1c8b6 100644
The "index" line is still showing that there _is_ a difference.
The --ignore-* options are there merely to tell git what changes are not
worth _showing_ in the textual part of the patch, in order to cut down the
amount of the output. It never affects the outcome.
So if anything, I think --no-index codepath is what's buggy; if it does
not report the blob difference that is a different matter, though.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-05-22 16:14 ` Junio C Hamano
@ 2009-05-22 17:54 ` Jim Meyering
2009-05-22 20:40 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Jim Meyering @ 2009-05-22 17:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
Junio C Hamano wrote:
> Jim Meyering <jim@meyering.net> writes:
>> git-diff's --quiet option works how I'd expect with --ignore-space-at-eol
>> as long as I'm also using --no-index:
>>
>> $ echo>b; echo \ >c; git diff --no-index --quiet --ignore-space-at-eol b c \
>> && echo good
>> good
>>
>> But in what I think of as normal operation (i.e., without --no-index),
>> --exit-code (or --quiet) makes git-diff say there were differences,
>> even when they have been ignored:
>>
>> # do this in an empty directory
>> $ git init -q; echo>k; git add .; git commit -q -m. .; echo \ >k
>> $ git diff --ignore-space-at-eol --quiet || echo bad
>> bad
>
> I am slightly torn about this, in that I can picture myself saying that
> this is unintuitive on some different days, but not today ;-)
Thanks for the quick reply. Here's why I noticed:
I wanted to ensure that the only changes induced by commit C were
to trailing blanks. I wrote something like this, expecting to be able
to deal with the exception:
git --quiet --ignore-space-at-eol --quiet C^..C || handle_unexpected
But handle_unexpected was always being invoked.
I was surprised because GNU diff's --ignore-all-space (-w) option does
work the way I expected:
$ echo>b; echo \ >c; diff -w b c && echo $?
0
> If you look at the output (i.e. no --quiet), you would see that the blob
> changes are still reported for the path. E.g. you would see something
> like...
>
> $ git diff --ignore-space-at-eol
> diff --git a/k b/k
> index 8b13789..8d1c8b6 100644
>
> The "index" line is still showing that there _is_ a difference.
I did see that, to my chagrin:
if using a --ignore-... option had also suppressed those, I could
have tested for empty output instead of exit status.
> The --ignore-* options are there merely to tell git what changes are not
> worth _showing_ in the textual part of the patch, in order to cut down the
> amount of the output. It never affects the outcome.
>
> So if anything, I think --no-index codepath is what's buggy; if it does
> not report the blob difference that is a different matter, though.
If need be, I can work around it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-05-22 17:54 ` Jim Meyering
@ 2009-05-22 20:40 ` Junio C Hamano
2009-05-23 7:26 ` Jim Meyering
2009-08-30 16:25 ` Jim Meyering
0 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2009-05-22 20:40 UTC (permalink / raw)
To: Jim Meyering; +Cc: Junio C Hamano, git list
Jim Meyering <jim@meyering.net> writes:
> Junio C Hamano wrote:
>> Jim Meyering <jim@meyering.net> writes:
>>>
>>> # do this in an empty directory
>>> $ git init -q; echo>k; git add .; git commit -q -m. .; echo \ >k
>>> $ git diff --ignore-space-at-eol --quiet || echo bad
>>> bad
>>
>> I am slightly torn about this, in that I can picture myself saying that
>> this is unintuitive on some different days, but not today ;-)
>
> Thanks for the quick reply. Here's why I noticed:
> ...
It seems that today is already "some different day" ;-) We could do
something like this patch.
While in the longer term I think it may make the world a better place by
being more consistent with what users expect, I am not sure at what
revision boundary we should introduce such a semantic change.
We could always declare this a bug and apply the "fix" at any time. It's
all perception ;-).
-- >8 --
Subject: [PATCH] diff --quiet: special case "ignore whitespace" options
The option "QUIET" primarily meant "find if we have _any_ difference as
quick as possible and report", which means we often do not even have to
look at blobs if we know the trees are different by looking at the higher
level (e.g. "diff-tree A B"). As a side effect, because there is no point
showing one change that we happened to have found first, it also enables
NO_OUTPUT and EXIT_WITH_STATUS options, making the end result look quiet.
Traditionally, the --ignore-whitespace* options have merely meant to tell
the diff output routine that some class of differences are not worth
showing in the textual diff output, so that the end user has easier time
to review the remaining (presumably more meaningful) changes. These
options never affected the outcome of the command, given as the exit
status when the --exit-code option was in effect (either directly or
indirectly).
These two classes of options are incompatible. When you have only
whitespace changes, you would expect:
git diff -b --quiet
to report that there is _no_ change. This is unfortunately not the case,
however, if there are differences to be reported if the command was run
without --quiet; there _is_ a change, and the command still exits with
non-zero status.
And that is wrong.
Change the semantics of --ignore-whitespace* options to mean more than
"omit showing the difference in text". When these options are used, the
internal "quick" optimization is turned off, and the status reported with
the --exit-code option will now match if any the textual diff output is
actually produced.
Also rename the internal option "QUIET" to "QUICK" to better reflect what
its true purpose is.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-log.c | 2 +-
builtin-rev-list.c | 2 +-
diff-lib.c | 4 ++--
diff.c | 39 ++++++++++++++++++++++++++++++++++++---
diff.h | 3 ++-
revision.c | 2 +-
tree-diff.c | 2 +-
7 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 5eaec5d..80624f5 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -536,7 +536,7 @@ static int reopen_stdout(struct commit *commit, struct rev_info *rev)
get_patch_filename(commit, rev->nr, fmt_patch_suffix, &filename);
- if (!DIFF_OPT_TST(&rev->diffopt, QUIET))
+ if (!DIFF_OPT_TST(&rev->diffopt, QUICK))
fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
if (freopen(filename.buf, "w", stdout) == NULL)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 38a8f23..61d3126 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -315,7 +315,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
memset(&info, 0, sizeof(info));
info.revs = &revs;
- quiet = DIFF_OPT_TST(&revs.diffopt, QUIET);
+ quiet = DIFF_OPT_TST(&revs.diffopt, QUICK);
for (i = 1 ; i < argc; i++) {
const char *arg = argv[i];
diff --git a/diff-lib.c b/diff-lib.c
index a310fb2..a549ee6 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -73,7 +73,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
struct cache_entry *ce = active_cache[i];
int changed;
- if (DIFF_OPT_TST(&revs->diffopt, QUIET) &&
+ if (DIFF_OPT_TST(&revs->diffopt, QUICK) &&
DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
break;
@@ -520,7 +520,7 @@ int index_differs_from(const char *def, int diff_flags)
init_revisions(&rev, NULL);
setup_revisions(0, NULL, &rev, def);
- DIFF_OPT_SET(&rev.diffopt, QUIET);
+ DIFF_OPT_SET(&rev.diffopt, QUICK);
DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);
rev.diffopt.flags |= diff_flags;
run_diff_index(&rev, 1);
diff --git a/diff.c b/diff.c
index f06876b..f2ed2ac 100644
--- a/diff.c
+++ b/diff.c
@@ -2370,6 +2370,21 @@ int diff_setup_done(struct diff_options *options)
if (count > 1)
die("--name-only, --name-status, --check and -s are mutually exclusive");
+ /*
+ * Most of the time we can say "there are changes"
+ * only by checking if there are changed paths, but
+ * --ignore-whitespace* options force us to look
+ * inside contets.
+ */
+
+ if ((XDF_IGNORE_WHITESPACE|
+ XDF_IGNORE_WHITESPACE_CHANGE|
+ XDF_IGNORE_WHITESPACE_AT_EOL) & options->xdl_opts) {
+ DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
+ } else {
+ DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
+ }
+
if (DIFF_OPT_TST(options, FIND_COPIES_HARDER))
options->detect_rename = DIFF_DETECT_COPY;
@@ -2430,9 +2445,19 @@ int diff_setup_done(struct diff_options *options)
* to have found. It does not make sense not to return with
* exit code in such a case either.
*/
- if (DIFF_OPT_TST(options, QUIET)) {
+ if (DIFF_OPT_TST(options, QUICK)) {
options->output_format = DIFF_FORMAT_NO_OUTPUT;
DIFF_OPT_SET(options, EXIT_WITH_STATUS);
+
+ /*
+ * QUICK means "if we find any difference return early
+ * and say 'there is a difference'", and we often do
+ * not even look at the blobs. Some options would not
+ * be compatible with this optimization, so we turn it
+ * off, make it into "no output but exit with status".
+ */
+ if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
+ DIFF_OPT_CLR(options, QUICK);
}
return 0;
@@ -2621,7 +2646,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
else if (!strcmp(arg, "--exit-code"))
DIFF_OPT_SET(options, EXIT_WITH_STATUS);
else if (!strcmp(arg, "--quiet"))
- DIFF_OPT_SET(options, QUIET);
+ /* see postprocessing in diff_setup_done() */
+ DIFF_OPT_SET(options, QUICK);
else if (!strcmp(arg, "--ext-diff"))
DIFF_OPT_SET(options, ALLOW_EXTERNAL);
else if (!strcmp(arg, "--no-ext-diff"))
@@ -3322,6 +3348,13 @@ free_queue:
q->nr = q->alloc = 0;
if (options->close_file)
fclose(options->file);
+
+ if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
+ if (options->found_changes)
+ DIFF_OPT_SET(options, HAS_CHANGES);
+ else
+ DIFF_OPT_CLR(options, HAS_CHANGES);
+ }
}
static void diffcore_apply_filter(const char *filter)
@@ -3458,7 +3491,7 @@ void diffcore_std(struct diff_options *options)
diff_resolve_rename_copy();
diffcore_apply_filter(options->filter);
- if (diff_queued_diff.nr)
+ if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
DIFF_OPT_SET(options, HAS_CHANGES);
else
DIFF_OPT_CLR(options, HAS_CHANGES);
diff --git a/diff.h b/diff.h
index 6616877..a7e7ccb 100644
--- a/diff.h
+++ b/diff.h
@@ -55,7 +55,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_COLOR_DIFF (1 << 8)
#define DIFF_OPT_COLOR_DIFF_WORDS (1 << 9)
#define DIFF_OPT_HAS_CHANGES (1 << 10)
-#define DIFF_OPT_QUIET (1 << 11)
+#define DIFF_OPT_QUICK (1 << 11)
#define DIFF_OPT_NO_INDEX (1 << 12)
#define DIFF_OPT_ALLOW_EXTERNAL (1 << 13)
#define DIFF_OPT_EXIT_WITH_STATUS (1 << 14)
@@ -66,6 +66,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_DIRSTAT_CUMULATIVE (1 << 19)
#define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
#define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)
+#define DIFF_OPT_DIFF_FROM_CONTENTS (1 << 22)
#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/revision.c b/revision.c
index 18b7ebb..1c114ab 100644
--- a/revision.c
+++ b/revision.c
@@ -800,7 +800,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
revs->ignore_merges = 1;
revs->simplify_history = 1;
DIFF_OPT_SET(&revs->pruning, RECURSIVE);
- DIFF_OPT_SET(&revs->pruning, QUIET);
+ DIFF_OPT_SET(&revs->pruning, QUICK);
revs->pruning.add_remove = file_add_remove;
revs->pruning.change = file_change;
revs->lifo = 1;
diff --git a/tree-diff.c b/tree-diff.c
index edd8394..ac85a55 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -280,7 +280,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru
int baselen = strlen(base);
for (;;) {
- if (DIFF_OPT_TST(opt, QUIET) && DIFF_OPT_TST(opt, HAS_CHANGES))
+ if (DIFF_OPT_TST(opt, QUICK) && DIFF_OPT_TST(opt, HAS_CHANGES))
break;
if (opt->nr_paths) {
skip_uninteresting(t1, base, baselen, opt);
--
1.6.3.1.70.ga80aa
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-05-22 20:40 ` Junio C Hamano
@ 2009-05-23 7:26 ` Jim Meyering
2009-08-30 16:25 ` Jim Meyering
1 sibling, 0 replies; 9+ messages in thread
From: Jim Meyering @ 2009-05-23 7:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
Junio C Hamano wrote:
> Jim Meyering <jim@meyering.net> writes:
>> Junio C Hamano wrote:
>>> Jim Meyering <jim@meyering.net> writes:
>>>>
>>>> # do this in an empty directory
>>>> $ git init -q; echo>k; git add .; git commit -q -m. .; echo \ >k
>>>> $ git diff --ignore-space-at-eol --quiet || echo bad
>>>> bad
>>>
>>> I am slightly torn about this, in that I can picture myself saying that
>>> this is unintuitive on some different days, but not today ;-)
>>
>> Thanks for the quick reply. Here's why I noticed:
>> ...
>
> It seems that today is already "some different day" ;-) We could do
> something like this patch.
>
> While in the longer term I think it may make the world a better place by
> being more consistent with what users expect, I am not sure at what
> revision boundary we should introduce such a semantic change.
>
> -- >8 --
> Subject: [PATCH] diff --quiet: special case "ignore whitespace" options
> ...
Wow. And now a patch. Service with style ;-)
> We could always declare this a bug and apply the "fix" at any time. It's
> all perception ;-).
The declare-it-a-bug option sounds sensible, since I doubt anyone
even noticed, much less relied on, the changing behavior.
Thank you!
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-05-22 20:40 ` Junio C Hamano
2009-05-23 7:26 ` Jim Meyering
@ 2009-08-30 16:25 ` Jim Meyering
2009-08-30 20:11 ` Junio C Hamano
1 sibling, 1 reply; 9+ messages in thread
From: Jim Meyering @ 2009-08-30 16:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
Junio C Hamano wrote:
> Jim Meyering <jim@meyering.net> writes:
>> Junio C Hamano wrote:
>>> Jim Meyering <jim@meyering.net> writes:
>>>>
>>>> # do this in an empty directory
>>>> $ git init -q; echo>k; git add .; git commit -q -m. .; echo \ >k
>>>> $ git diff --ignore-space-at-eol --quiet || echo bad
>>>> bad
>>>
>>> I am slightly torn about this, in that I can picture myself saying that
>>> this is unintuitive on some different days, but not today ;-)
>>
>> Thanks for the quick reply. Here's why I noticed:
>> ...
>
> It seems that today is already "some different day" ;-) We could do
> something like this patch.
>
> While in the longer term I think it may make the world a better place by
> being more consistent with what users expect, I am not sure at what
> revision boundary we should introduce such a semantic change.
>
> We could always declare this a bug and apply the "fix" at any time. It's
> all perception ;-).
>
> -- >8 --
> Subject: [PATCH] diff --quiet: special case "ignore whitespace" options
>
> The option "QUIET" primarily meant "find if we have _any_ difference as
> quick as possible and report", which means we often do not even have to
> look at blobs if we know the trees are different by looking at the higher
> level (e.g. "diff-tree A B"). As a side effect, because there is no point
> showing one change that we happened to have found first, it also enables
> NO_OUTPUT and EXIT_WITH_STATUS options, making the end result look quiet.
>
> Traditionally, the --ignore-whitespace* options have merely meant to tell
> the diff output routine that some class of differences are not worth
> showing in the textual diff output, so that the end user has easier time
> to review the remaining (presumably more meaningful) changes. These
> options never affected the outcome of the command, given as the exit
> status when the --exit-code option was in effect (either directly or
> indirectly).
>
> These two classes of options are incompatible. When you have only
> whitespace changes, you would expect:
>
> git diff -b --quiet
>
> to report that there is _no_ change. This is unfortunately not the case,
> however, if there are differences to be reported if the command was run
> without --quiet; there _is_ a change, and the command still exits with
> non-zero status.
>
> And that is wrong.
>
> Change the semantics of --ignore-whitespace* options to mean more than
> "omit showing the difference in text". When these options are used, the
> internal "quick" optimization is turned off, and the status reported with
> the --exit-code option will now match if any the textual diff output is
> actually produced.
>
> Also rename the internal option "QUIET" to "QUICK" to better reflect what
> its true purpose is.
Thanks again.
If there's anything I can to do help (add a test?), let me know.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-08-30 16:25 ` Jim Meyering
@ 2009-08-30 20:11 ` Junio C Hamano
2009-08-30 20:27 ` Jim Meyering
2009-09-08 20:58 ` Thell Fowler
0 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2009-08-30 20:11 UTC (permalink / raw)
To: Jim Meyering; +Cc: git list
Jim Meyering <jim@meyering.net> writes:
> Junio C Hamano wrote:
> ...
>> Subject: [PATCH] diff --quiet: special case "ignore whitespace" options
>> ...
>> Change the semantics of --ignore-whitespace* options to mean more than
>> "omit showing the difference in text". When these options are used, the
>> internal "quick" optimization is turned off, and the status reported with
>> the --exit-code option will now match if any the textual diff output is
>> actually produced.
>>
>> Also rename the internal option "QUIET" to "QUICK" to better reflect what
>> its true purpose is.
>
> Thanks again.
> If there's anything I can to do help (add a test?), let me know.
The change has been cooking in 'next' and hopefully be in 1.7.0. I think
the updated series adds its own test script, too.
Using it in every day scenario, and reporting any breakage you notice
before 1.7.0 happens, would be greatly appreciated.
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-08-30 20:11 ` Junio C Hamano
@ 2009-08-30 20:27 ` Jim Meyering
2009-09-08 20:58 ` Thell Fowler
1 sibling, 0 replies; 9+ messages in thread
From: Jim Meyering @ 2009-08-30 20:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
Junio C Hamano wrote:
> Jim Meyering <jim@meyering.net> writes:
>> Junio C Hamano wrote:
>> ...
>>> Subject: [PATCH] diff --quiet: special case "ignore whitespace" options
>>> ...
>>> Change the semantics of --ignore-whitespace* options to mean more than
>>> "omit showing the difference in text". When these options are used, the
>>> internal "quick" optimization is turned off, and the status reported with
>>> the --exit-code option will now match if any the textual diff output is
>>> actually produced.
>>>
>>> Also rename the internal option "QUIET" to "QUICK" to better reflect what
>>> its true purpose is.
>>
>> Thanks again.
>> If there's anything I can to do help (add a test?), let me know.
>
> The change has been cooking in 'next' and hopefully be in 1.7.0. I think
> the updated series adds its own test script, too.
>
> Using it in every day scenario, and reporting any breakage you notice
> before 1.7.0 happens, would be greatly appreciated.
Oh! I am using next (will test!), and even searched log summary output,
but obviously my search was too cursory or just inaccurate.
I glanced through it and it looks fine (of course!).
I spotted one typo, and suggest a second change that's barely worth
mentioning, both in comments:
diff --git a/diff.c b/diff.c
index 91d6ea2..24bd3fc 100644
--- a/diff.c
+++ b/diff.c
@@ -2382,7 +2382,7 @@ int diff_setup_done(struct diff_options *options)
* Most of the time we can say "there are changes"
* only by checking if there are changed paths, but
* --ignore-whitespace* options force us to look
- * inside contets.
+ * inside contents.
*/
if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
@@ -3346,7 +3346,7 @@ free_queue:
fclose(options->file);
/*
- * Report the contents level differences with HAS_CHANGES;
+ * Report the content-level differences with HAS_CHANGES;
* diff_addremove/diff_change does not set the bit when
* DIFF_FROM_CONTENTS is in effect (e.g. with -w).
*/
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: git-diff: must --exit-code work with --ignore* options?
2009-08-30 20:11 ` Junio C Hamano
2009-08-30 20:27 ` Jim Meyering
@ 2009-09-08 20:58 ` Thell Fowler
1 sibling, 0 replies; 9+ messages in thread
From: Thell Fowler @ 2009-09-08 20:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jim Meyering, git list
On Sun, 30 Aug 2009, Junio C Hamano wrote:
> Jim Meyering <jim@meyering.net> writes:
>
>> Junio C Hamano wrote:
>> ...
>>> Subject: [PATCH] diff --quiet: special case "ignore whitespace" options
>>> ...
>>> Change the semantics of --ignore-whitespace* options to mean more than
>>> "omit showing the difference in text". When these options are used, the
>>> internal "quick" optimization is turned off, and the status reported with
>>> the --exit-code option will now match if any the textual diff output is
>>> actually produced.
>>>
>>> Also rename the internal option "QUIET" to "QUICK" to better reflect what
>>> its true purpose is.
>>
>> Thanks again.
>> If there's anything I can to do help (add a test?), let me know.
>
> The change has been cooking in 'next' and hopefully be in 1.7.0. I think
> the updated series adds its own test script, too.
>
> Using it in every day scenario, and reporting any breakage you notice
> before 1.7.0 happens, would be greatly appreciated.
>
> Thanks.
Perhaps I'm expected something different than what I _should_ be
expecting, but shouldn't --quiet always return the same as --exit-code?
# Cut/Paste example
mkdir test_ws_quiet && cd test_ws_quiet && git init
printf "foo bar \n\n" >f1.txt
git add .
git commit -m 'f text'
printf "foo bar\n\n" >f1.txt
git commit -a -m 'f with diff white-space in middle & end'
git diff -w --exit-code HEAD^ >/dev/null
echo $?
# returns '0' which it should
git diff -w --quiet HEAD^
echo $?
# returns '0' which it should
git diff -b --exit-code HEAD^ >/dev/null
echo $?
# returns '0' which it should
git diff -b --quiet HEAD^ >/dev/null
echo $?
# returns '0' which it should
git diff --ignore-space-at-eol --exit-code HEAD^ >/dev/null
echo $?
# returns '1' which it should
git diff --ignore-space-at-eol --quiet HEAD^
echo $?
#returns '0' <=== Unexpected.
#
# Next phase
#
printf "foobar\n\n">f1.txt
git commit -a -m 'f without any spaces'
git diff -w --exit-code HEAD^ >/dev/null
echo $?
# returns '0' which it should
git diff -w --quiet HEAD^
echo $?
# returns '0' which it should
git diff -b --exit-code HEAD^ >/dev/null
echo $?
# returns '1' which it should
git diff -b --quiet HEAD^ >/dev/null
echo $?
# returns '0' <=== Unexpected
git diff --ignore-space-at-eol --exit-code HEAD^ >/dev/null
echo $?
# returns '1' which it should
git diff --ignore-space-at-eol --quiet HEAD^
echo $?
#returns '0' <=== Unexpected.
--
Thell
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-09-08 20:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-22 14:01 git-diff: must --exit-code work with --ignore* options? Jim Meyering
2009-05-22 16:14 ` Junio C Hamano
2009-05-22 17:54 ` Jim Meyering
2009-05-22 20:40 ` Junio C Hamano
2009-05-23 7:26 ` Jim Meyering
2009-08-30 16:25 ` Jim Meyering
2009-08-30 20:11 ` Junio C Hamano
2009-08-30 20:27 ` Jim Meyering
2009-09-08 20:58 ` Thell Fowler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).