Re: [PATCH 10/12] builtin/show-ref: explicitly spell out different modes in synopsis

On Tue, Oct 24, 2023 at 03:39:28PM -0400, Eric Sunshine wrote:

On Tue, Oct 24, 2023 at 9:11 AM Patrick Steinhardt [off-list ref] wrote:

quoted

The synopsis treats the `--verify` and the implicit mode the same. They
are slightly different though:

    - They accept different sets of flags.

    - The implicit mode accepts patterns while the `--verify` mode
      accepts references.

Split up the synopsis for these two modes such that we can disambiguate
those differences.

Good. When reading [2/12], my immediate thought was that such a
documentation change was needed.

quoted

Signed-off-by: Patrick Steinhardt <redacted>
---

diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt

@@ -8,9 +8,12 @@ git-show-ref - List references in a local repository
 SYNOPSIS
-'git show-ref' [-q | --quiet] [--verify] [--head] [-d | --dereference]
+'git show-ref' [-q | --quiet] [--head] [-d | --dereference]
             [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]
             [--heads] [--] [<pattern>...]
+'git show-ref' --verify [-q | --quiet] [-d | --dereference]
+            [-s | --hash[=<n>]] [--abbrev[=<n>]]
+            [--] [<ref>...]
 'git show-ref' --exclude-existing[=<pattern>]

What does it mean to request "quiet" for the plain `git show-ref`
mode? That seems pointless and counterintuitive. Even though this mode
may accept --quiet as a quirk of implementation, we probably shouldn't
be promoting its use in the documentation. Moreover, the blurb for
--quiet later in the document:

   Do not print any results to stdout. When combined with --verify,
   this can be used to silently check if a reference exists.

should probably be rephrased since it currently implies that it may be
used with modes other than --verify, but that's not really the case
(implementation quirks aside).

Good point indeed, will change.

This also raises the question as to whether an interlock should be
added to disallow --quiet with plain `git show-ref`, much like the
interlock preventing --exclude-existing and --verify from being used
together. Ideally, such an interlock ought to be added, but I wouldn't
be surprised to learn that doing so would break someone's existing
tooling which insensibly uses --quiet with plain `git show-ref`.

Yeah, I also wouldn't go as far as this. The mutual exclusiveness for
`--exclude-existing` and `--verify` makes sense in my opinion because
the result is extremely misleading and may cause users to assume that
the wrong thing has happened.

I don't think that's necessarily true for `--quiet`. It may not make a
lot of sense to specify `--quiet` here, but it also doesn't quietly do
the wrong thing as in the other case.

Furthermore, we also don't have any interlocks for incompatible other
flags, either: git-show-ref(1) won't complain when passing any of the
mode-specific flags to the other modes. If we want to fix that I'd
rather defer it to a follow up patch series though. And as you said, I
would almost certainly expect there to be some kind of fallout if we did
this change.

Patrick