Re: [PATCH v2] worktree: add --quiet option

["Martin Ågren" <martin.agren@gmail.com>]

On Wed, 15 Aug 2018 at 22:56, Elia Pinto <gitter.spiros@gmail.com> wrote:
> Add the '--quiet' option to git worktree,
> as for the other git commands. 'add' is the
> only command affected by it since all other
> commands, except 'list', are currently
> silent by default.

Thanks for a follow-up.

The word "currently" means I can't shake the feeling that Eric has a
very good point in [1]:

  It might make sense to say instead that this is adding a --quiet
  option _in general_, rather than doing so specifically for 'add'.

As an example, if `git worktree move` ever learns to produce some sort
of output, then Eric's approach would mean that such a hypothetical
`move` is buggy until it learns to respect `--quiet`. With your
approach, it would mean that we would get feature requests that `move`
should also respect `--quiet` (which we would then need to redefine in
the documentation) or that it should learn of a `--quiet-move` (which I
do not think is a particularly good UI).

Doing such a patch instead would mean tweaking the commit message
slightly...

  Add the '--quiet' option to git worktree, as for the other git
  commands. Currently, 'add' is the only command affected by it since
  all other commands, except 'list' obviously, are already silent by
  default.

... and the documentation slightly ...

  Suppress feedback messages.

It might make sense adding a comment to the documentation about how this
currently only affects `add`, but such comments do risk going stale. I
am on the fence about whether the documentation needs to say something
about `list` -- who in their right mind would expect `list --quiet` to
actually suppress the list? And if `list` ever learns to give some
feedback messages in addition to the list itself, then we would want
`--quiet` to suppress *those*, I guess.

Others might disagree violently with this, but I wonder whether it makes
sense to add a test to verify that, e.g., `git worktree move --quiet` is
quiet. Such a test would demonstrate that this is your intention, i.e.,
anyone digging into a report on "why does git worktree foo not respect
--quiet?" would be 100% convinced that your intention back in 2018 really
was to respect it everywhere. It seems wasteful to add such a test for
all other modes, but maybe you can identify one which you think has a
higher risk of learning to output some feedback in the future.

To be clear, it is fine for you to disagree with the above! :-)

>         }
> -
> -       print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
> +       if (!opts.quiet)
> +               print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);

I think that empty line should be kept. Probably not worth a reroll.

Good work!

[1] https://public-inbox.org/git/CAPig+cS-b2yL2FeLRzS+jW-O5fRd1g8Kqak7j1QX5PRP0ojQEQ@mail.gmail.com/

Martin