Re: git-diff: must --exit-code work with --ignore* options?

Junio C Hamano wrote:

Jim Meyering [off-list ref] writes:

quoted

Junio C Hamano wrote:

quoted

Jim Meyering [off-list ref] writes:

quoted

    # 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.

Jim Meyering [off-list ref] writes:

Junio C Hamano wrote:
...

quoted

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.

On Sun, 30 Aug 2009, Junio C Hamano wrote:

Jim Meyering [off-list ref] writes:

quoted

Junio C Hamano wrote:
...

quoted

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