Re: [PATCH v1 01/11] checkout: split part of it to new command 'restore'

[Duy Nguyen <pclouds@gmail.com>]

(since 'git switch' is basically done, let's get back to 'git restore')

On Sun, Mar 10, 2019 at 1:27 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Sat, Mar 9, 2019 at 4:16 AM Duy Nguyen <pclouds@gmail.com> wrote:
> > On Sat, Mar 9, 2019 at 1:01 AM Elijah Newren <newren@gmail.com> wrote:
>
> > > > +-q::
> > > > +--quiet::
> > > > +       Quiet, suppress feedback messages.
> > > > +
> > > > +--progress::
> > > > +--no-progress::
> > > > +       Progress status is reported on the standard error stream
> > > > +       by default when it is attached to a terminal, unless `--quiet`
> > > > +       is specified. This flag enables progress reporting even if not
> > > > +       attached to a terminal, regardless of `--quiet`.
> > >
> > > I'm assuming this means there are feedback messages other than
> > > progress feedback?
> >
> > There could be. This is carried over from git-checkout. I suspect this
> > is about warnings that we print from time to time.
>
> Why would --quiet squelch warnings?  I figured it'd only squelch
> feedback, informational, or progress messages.

I'm wrong. Warnings are not suppressed by --quiet. It looks like
--quiet is not really used by 'git restore'. Based on code inspection,
--quiet implies --no-progress if --[no-]progress is not specified.
Otherwise progress bar is enabled only when we have tty.

> I understand you just carried it over from git-checkout, but as worded
> it makes me wonder if checkout has suboptimal behavior or perhaps just
> a suboptimal explanation of its flags ... and if it does, we probably
> don't want to carry that over.

But there is something we don't show now, but I don't know if we should show.

The commit that started all this is 0f086e6dca (checkout: print
something when checking out paths, 2018-11-13) where I tried to make
'git checkout' mistakes easier to see by printing

    Updated <n> paths from <some-checkout-source>

Because the reason is to help spot mistakes, this is only printed when
you do "git checkout <paths>" _without_ the double dashes to
disambiguate.

Since git-restore has no ambiguation (between restoring paths and
switching branches) these messages are never printed. I'm just
wondering if we should print them anyway. It gives some feedback
instead of the silent output in the successful case. Some might like
it, some don't.

> > > > +-f::
> > > > +--force::
> > > > +       If `--source` is not specified, unmerged entries are left alone
> > > > +       and will not fail the operation. Unmerged entries are always
> > > > +       replaced if `--source` is specified, regardless of `--force`.
> > >
> > > This may be slightly confusing, in particular it suggests that --index
> > > (or --worktree and --index) are the default.  Is --force only useful
> > > when --index is specified?  If it has utility with --worktree only,
> > > what does it do?
> >
> > Well, this is 'git checkout -f' behavior which only concerns the
> > index. So yeah it only matters with --index.
>
> Okay, good to know that this only matters with --index. However, new
> problem: This makes the explanation feel contradictory, though,
> because elsewhere you stated that --source=HEAD is implied when
> --index is given without a source.  So, the combination of this
> description and that fact suggests that -f is either useless (--index
> is not specified) or ignored (because --source will either default to
> HEAD or be specified by the user).  Maybe that's true and -f should be
> removed from this new document.  If it has actual utility in some
> cases, then this description needs to be reworked to explain what
> those circiumstances are somehow.

Actually my description of this option is not clear. This is about
--worktree, not --index.

When we do "git restore <path>" (which by default implies --worktree),
we take the entry from the index and overwrite the worktree with it.
But if the entry is unmerged (and --ours or --theirs are not
specified), there is no good way we can handle it automatically.

So the default behavior in this case is abort the operation. -f goes
with a gentler option: ignore unmerged entries and go restore normal
ones. And to keep people informed that there _are_ unmerged entries,
warning is displayed for each one.

So, maybe I'll rename --force to --ignore-unmerged? This option will
only matter in the "git restore [--worktree]" form. In other words,
restoring worktree from the index. If --ignored-unmerged is specified
in other modes, die().
-- 
Duy