Re: [PATCH 2/2] restore: default to HEAD when combining --worktree and --staged

From: Taylor Blau <me@ttaylorr.com>
Date: May 5, 2020, 04:44 UTC
Message-ID: <20200505044449.GA67132@syl.local>
In-Reply-To: <CAPig+cTAab6E4TPLL5AA75Ss9hQxCo2ZKLQaDakO=gGPvse+cQ@mail.gmail.com>

On Tue, May 05, 2020 at 12:00:08AM -0400, Eric Sunshine wrote:

Show 10 quoted lines

> On Fri, May 1, 2020 at 6:20 PM Taylor Blau <me@ttaylorr.com> wrote:
> > On Fri, May 01, 2020 at 04:27:46AM -0400, Eric Sunshine wrote:
> > > The default restore source for --worktree is the index, and the default
> > > source for --staged is HEAD. However, when combining --worktree and
> >
> > I think that you could very reasonably drop the first sentence here,
> > especially because it is repeated verbatim from the previous commit.
>
> The repetition is intentional so that each commit can be understood
> stand-alone (without having to know what came before it).

Fair enough; I figure that the commits will probably be read most often in conjunction with one another, but a little bit of extra commentary doesn't hurt, either.

Show 7 quoted lines

> > In fact... this whole paragraph looks similar to me. Maybe just:
> >
> >  When invoking 'git restore' with both '--worktree' and '--staged', it
> >  is required that the ambiguity of which source to restore from be
> >  resolved by also passing '--source'.
>
> I'll see if I can trim it down a bit -- Junio also found it too long.

Thanks. I'm happy to read whatever you have once you're ready.

Show 16 quoted lines

> > > -By default, the restore sources for working tree and the index are the
> > > -index and `HEAD` respectively. `--source` could be used to specify a
> > > -commit as the restore source; it is required when combining `--staged`
> > > -and `--worktree`.
> > > +By default, the restore source for `--worktree` is the index, and the
> > > +restore source for `--staged` is `HEAD`. When combining `--worktree` and
> > > +`--staged`, the restore source is `HEAD`. `--source` can be used to specify
> >
> > This is extremely nit-pick-y, but is this line a little over-long? My
> > memory is that Documentation should be wrapped at 72 characters instead
> > of 80. I culd be totally wrong.
>
> Column 72 for commit messages, certainly, but I don't think there is
> any such guideline about documentation also being wrapped at 72. As an
> old-schooler who still uses 80-column terminal and editor windows, I'm
> quite sensitive to line length -- these lines are wrapped at 79.

Hmm... I've always wrapped changes in the Documentation tree at 72 characters, but I could very easily be in the wrong there ;). I tried to find something in Documentation about wrapping at 72 characters, but I failed. So, please disregard my original suggestion, and sorry for the trouble there.

Thanks, Taylor