Can we clarify the purpose of `git diff -s`?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



A recent discussion about a bug in the diff machinery [1] made me dig
the history of the `-s` option, and turned out to be quite an
archeological endeavor.

The first indication of such flag comes from e2e5e98a40 ([PATCH] Silent
flag for show-diff, 2005-04-13), only 54 commits after the initial
commit.

Not much later after, a `-z` option was added for some machine-readable
output in d94c6128e6 ([PATCH] show-diff -z option for machine readable
output., 2005-04-16).

Linus Torvalds wanted to make the machine-readable output the only one
and wrote 0a7668e99f (show-diff: match diff-tree and diff-cache output,
2005-04-26), but Junio Hamano disagreed and added a `-p` option for
a human-readable patch in 4a6bf9e18a ([PATCH] Reactivate show-diff patch
generation, 2005-04-27).

  You'll need "diff-tree-helper" to show the full diff, but Junio is
  dead set on adding a "-p" argument to all three to avoid it. That's
  next..

Right after that, Junio Hamano deprecated the `-s` flag, since
`git-show-diff` didn't show the patch by default, so `-s` became a
no-op. I presume at that point in time they didn't think of the
possibility of doing `-p -s` together.

The first introduction of DIFF_FORMAT_NO_OUTPUT was in a corner case of
6b14d7faf0 ([PATCH] Diffcore updates., 2005-05-22), but later on it was
used explicitly to replace a global variable of `git-diff-tree -s` in
d0309355c9 ([PATCH] Use DIFF_FORMAT_NO_OUTPUT to implement diff-tree -s
option., 2005-05-24).

When the equivalent of the modern `git diff` was added, the `-p` option
was included by default: 940c1bb018 (Add "git diff" script, 2005-06-13).
So later on when it was converted to C, DIFF_FORMAT_PATCH was the
default 65056021f2 (built-in diff., 2006-04-28).

But not for all commands, for example the default of `git diff-tree` is
DIFF_FORMAT_RAW, and it remains the case to this day.

So at this point it seems pretty clear that `-s` means `silent`, and
whatever the default format of a diff command is (`--patch` or `--raw`),
`-s` is meant to silence that format.

Later on in c6744349df (Merge with_raw, with_stat and summary variables
to output_format, 2006-06-24), the output format changed from an enum to
a bit field, so now it was possible to do for example
`DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW`.

This is when things become complicated, because now what is `-s`
supposed to do? Is it supposed to silence only the default format? Or is
it supposed to silence all formats?

For example, these two commands are equivalent:

  git diff-tree @~ @
  git diff-tree --raw @~ @

That's because the default format of `git diff-tree` is DIFF_FORMAT_RAW.

But what happens if we do:

  git diff-tree -s --patch @~ @

Shall we silence only the RAW part, or the PATCH part as well?

And then, should these two be different?

  git diff-tree --patch -s @~ @
  git diff-tree -s --patch @~ @

This is something that wasn't discussed or explored at the time, so it
is unclear.

And then, finally, we have d09cd15d19 (diff: allow --no-patch as synonym
for -s, 2013-07-16), which very clearly says:

  This follows the usual convention of having a --no-foo option to
  negate --foo.

So, obviously the intention of `--no-patch` is to negate `--patch`, but
it is linked to `-s`, which was linked to DIFF_FORMAT_NO_OUTPUT, which
means `--no-patch` negates *all* output, not just the output of
`--patch`.

So what should the output of this command be:

  git diff-tree --patch --no-patch --raw @~ @

I think it very clearly should output the same as:

  git diff-tree @~ @

And the ordering does not matter, as this should output the same:

  git diff-tree --raw --patch --no-patch @~ @

If we can combine two formats, for example:

  git diff --patch --raw @~

Then we should be able to negate a single format, for example:

  git diff --patch --raw --no-patch @~

Which in my mind should be different from:

  git diff --patch --raw --silent @~

So, in short: I don't think `-s` and `--no-patch` are the same thing at
all, and it was a mistake to link them together.

If anybody thinks the intention behind `-s` and `--no-patch` is
obviously clear, I think it would be helpful to explicitly say so for
the record.

Cheers.

[1] https://lore.kernel.org/git/20230503134118.73504-1-sorganov@xxxxxxxxx/

-- 
Felipe Contreras



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux