git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Stefan Beller <stefanbeller@gmail.com>
Subject: Re: [PATCH v1 01/11] checkout: split part of it to new command 'restore'
Date: Mon, 25 Mar 2019 08:51:04 -0700	[thread overview]
Message-ID: <CABPp-BFtsQzDj3mCr1aRTpy2PQCzATjGGccECCmH8Qa_Y4aASQ@mail.gmail.com> (raw)
In-Reply-To: <20190325095322.GA1617@ash>

On Mon, Mar 25, 2019 at 2:53 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> (Including Stefan, in case he's still interested in git and planned
> something for the "submodule" restore part I mention below)
>
> On Fri, Mar 08, 2019 at 10:01:25AM -0800, Elijah Newren wrote:
> > Thanks for working on this; overall looks really good.  I've got a few
> > comments here and there on the wording and combinations of options...
>
> OK now that the 'git switch' part is mostly settled, I could go back
> to 'git restore'. I have a lot more homework to do before sending v2,
> but how about this?
>
> Most of this is minor rephrasing and clarification (thanks!). The big
> change is `--force` is split into `--keep-unmerged` and
> `--force-submodules`, the same direction I took in 'git switch'.
>
> Submodules actually pose another problem because there are different
> things in a submodule to restore:
>
>  - restore HEAD only, leave submodule's worktree alone. This is
>    `--no-recurse-submodules` (default)
>
>  - restoring `HEAD`, which is basically "git switch --detach
>    <something>" inside the submodule. This is `--recurse-submodules`.
>
>  - restoring the paths inside the submodules, i.e. the actual "git
>    restore <some paths>" inside the submodule.
>
>  - a combinarion of two. Basically you want "git reset --hard" in
>    every selected submodule.
>
> I wonder if we ever need to cover the third and forth case (and if so,
> what will the UI be like).  I suppose we could eventually make
> --recurse-submodules take an argument to specify what to restore,
> maybe...

This looks pretty good to me, though I've got some small comments
below.  Note that I'm basically glossing over or ignoring any part
touching on submodules, so I could easily miss even glaring wording
problems in that part.

> PS. I just found https://asciidoclive.com/. Could be useful to paste
> this text and render without going through git's build system.

Interesting.  :-)

> -- 8< --
> git-restore(1)
> ==============
>
> NAME
> ----
> git-restore - Restore working tree files
>
> SYNOPSIS
> --------
> [verse]
> 'git restore' [<options>] [--source=<tree>] [--staged] [--worktree] <pathspec>...
> 'git restore' (-p|--patch) [<options>] [--source=<tree>] [--staged] [--worktree] [<pathspec>...]
>
> DESCRIPTION
> -----------
> Restore specified paths in the working tree with some contents from a
> restore source. If a path is tracked but does not exist in the restore
> source, it will be removed to match the source.
>
> The command can also be used to restore the content in the index with
> `--staged`, or restore both the working tree and the index with
> `--staged --worktree`.
>
> 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.
>
> A note about the differences between this command and linkgit:git-reset[1].
> `git restore` modifies the working tree (and maybe the index) to
> change file content to match some other, usually older, version, but
> does not update your branch. `git reset` is mainly about moving the
> tip of your branch in order to either add or remove commits from the
> branch.

Maybe include revert as well?  Perhaps something like...

A note about the differences between this command, linkgit:git-reset[1],
and and linkgit:git-revert[1].  `git restore` modifies the working tree
(and maybe the index) to change file content to match some other,
usually older, version, but does not update your branch. `git reset` is
mainly about moving the tip of your branch to some other commit,
implicitly adding and/or removing many commits to the branch.  `git
revert` is about creating and adding new commits to your branch that
reverse the changes made in specified commits.

(There is also a similar description in git-revert, which also
compares all three of revert, reset, and checkout.  It's shorter, but
focuses more from the "revert" angle.)

> OPTIONS
> -------
> -s <tree>::
> --source=<tree>::
>         Restore the working tree files with the content from the given
>         tree. It is common to specify the source tree by naming a
>         commit, branch or tag associated with it.
> +
> If not specified, the default restore source for the working tree is
> the index, and index `HEAD`. When both `--staged` and `--worktree`
> are specified, `--source` must also be specified.

s/index `HEAD`/the default restore source for the index is `HEAD`/

> -p::
> --patch::
>         Interactively select hunks in the difference between the
>         restore source and the restore location. See the ``Interactive
>         Mode'' section of linkgit:git-add[1] to learn how to operate
>         the `--patch` mode.
> +
> Note that `--patch` can accept no pathspec and will restore all
> modified paths.

s/will restore all/will prompt to restore with all/

> -W::
> --worktree::
> -S::
> --staged::
>         Specify the restore location. If neither option is specified,
>         by default the working tree is restored. Specifying `--staged`
>         will only restore the index. Specifying both restores both.
>
> -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`.
>
> -f::
> --force::
>         An alias of `--keep-unmerged` and `--force-submodules`.
>
> --keep-unmerged::
>         If `--source` is not specified, unmerged entries are left
>         alone and will not fail the operation.

This isn't very clear to me.  Maybe:

Incompatible with --source and --staged.  When specified, unmerged
entries in the index will not fail the operation and the corresponding
working tree files will be left unmodified.

> --force-submodules::
>         Continue even if restoring `HEAD` of a submodule leads to loss
>         of local changes in that submodule with `--recurse-submodules`.
>
> --ours::
> --theirs::
>         Restore from stage #2 ('ours') or #3 ('theirs') for unmerged
>         paths.
> +
> Note that during `git rebase` and `git pull --rebase`, 'ours' and
> 'theirs' may appear swapped. See the explanation of the same options
> in linkgit:git-checkout[1] for details.
>
> -m::
> --merge::
>         Recreate the conflicted merge in the specified paths.
>
> --conflict=<style>::
>         The same as `--merge` option above, but changes the way the
>         conflicting hunks are presented, overriding the merge.conflictStyle
>         configuration variable.  Possible values are "merge" (default)
>         and "diff3" (in addition to what is shown by "merge" style,
>         shows the original contents).
>
> --ignore-skip-worktree-bits::
>         In sparse checkout mode, by default is to only update entries
>         matched by `<pathspec>` and sparse patterns in
>         $GIT_DIR/info/sparse-checkout. This option ignores the sparse
>         patterns and unconditionally restores any files in
>         `<pathspec>`.
>
> --recurse-submodules::
> --no-recurse-submodules::
>         Using `--recurse-submodules` will update the content of all
>         initialized submodules according to the commit recorded in the
>         superproject. If nothing (or `--no-recurse-submodules`) is
>         used, the work trees of submodules will not be updated.  Just
>         like linkgit:git-submodule[1], this will detach `HEAD` of the
>         submodules.
>
> --overlay::
> --no-overlay::
>         In overlay mode, the command never removes files when
>         restoring. In no-overlay mode, tracked files that appear in
>         the index and working tree, but not in `--source` tree are
>         removed, to make them match `<tree>` exactly. The default is
>         no-overlay mode.

I think the "index and working tree" wording might cause some
problems.  What if the path doesn't exist in the source tree nor the
working tree, but does appear in the index and the user passes
--staged?  Maybe just remove that phrase, e.g.:

"...In no-overlay mode, tracked files that do not appear in the
`--source` tree are removed..."

?

> EXAMPLES
> --------
>
> The following sequence switches to the `master` branch, reverts the
> `Makefile` to two revisions back, deletes hello.c by mistake, and gets
> it back from the index.
>
> ------------
> $ git switch master
> $ git restore --source master~2 Makefile  <1>
> $ rm -f hello.c
> $ git restore hello.c                     <2>
> ------------
>
> <1> take a file out of another commit
> <2> restore hello.c from the index
>
> If you want to restore _all_ C source files to match the version in
> the index, you can say
>
> ------------
> $ git restore '*.c'
> ------------
>
> Note the quotes around `*.c`.  The file `hello.c` will also be
> restored, even though it is no longer in the working tree, because the
> file globbing is used to match entries in the index (not in the
> working tree by the shell).
>
> To restore all files in the current directory
>
> ------------
> $ git restore .
> ------------
>
> or to restore all working tree files with 'top' pathspec magic (see
> linkgit:gitglossary[7])
>
> ------------
> $ git restore :/
> ------------
>
> To restore a file in the index to match the version in `HEAD` (this is
> the same as using linkgit:git-reset[1])
>
> ------------
> $ git restore --staged hello.c
> ------------
>
> or you can restore both the index and the working tree (this the same
> as using linkgit:git-checkout[1])
>
> ------------
> $ git restore --source=HEAD --staged --worktree hello.c
> ------------
>
> or the short form which is more practical but less readable:
>
> ------------
> $ git restore -s@ -SW hello.c
> ------------

@ means HEAD?  I somehow missed this for five and half years.  (Glad
you put the "less readable" disclaimer/warning on this example,
though.)

  reply	other threads:[~2019-03-25 15:51 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-08 10:16 [PATCH v1 00/11] And new command "restore" Nguyễn Thái Ngọc Duy
2019-03-08 10:16 ` [PATCH v1 01/11] checkout: split part of it to new command 'restore' Nguyễn Thái Ngọc Duy
2019-03-08 18:01   ` Elijah Newren
2019-03-09 12:16     ` Duy Nguyen
2019-03-09 18:27       ` Elijah Newren
2019-04-09 12:18         ` Duy Nguyen
2019-03-25  9:53     ` Duy Nguyen
2019-03-25 15:51       ` Elijah Newren [this message]
2019-03-13  9:17   ` Johannes Schindelin
2019-03-08 10:16 ` [PATCH v1 02/11] restore: take tree-ish from --source option instead Nguyễn Thái Ngọc Duy
2019-03-09 18:13   ` Elijah Newren
2019-03-10  7:58   ` Eric Sunshine
2019-03-08 10:16 ` [PATCH v1 03/11] restore: make pathspec mandatory Nguyễn Thái Ngọc Duy
2019-03-09 18:35   ` Elijah Newren
2019-03-08 10:16 ` [PATCH v1 04/11] restore: disable overlay mode by default Nguyễn Thái Ngọc Duy
2019-03-09 18:37   ` Elijah Newren
2019-03-08 10:16 ` [PATCH v1 05/11] checkout: factor out worktree checkout code Nguyễn Thái Ngọc Duy
2019-03-08 10:16 ` [PATCH v1 06/11] restore: add --worktree and --index Nguyễn Thái Ngọc Duy
2019-03-09 18:52   ` Elijah Newren
2019-03-10 20:03     ` Eric Sunshine
2019-03-13 10:02     ` Duy Nguyen
2019-03-13 22:42     ` Junio C Hamano
2019-03-08 10:16 ` [PATCH v1 07/11] restore: default to --source=HEAD when only --index is specified Nguyễn Thái Ngọc Duy
2019-03-09 18:58   ` Elijah Newren
2019-03-08 10:16 ` [PATCH v1 08/11] restore: support --patch Nguyễn Thái Ngọc Duy
2019-03-09 19:02   ` Elijah Newren
2019-03-08 10:16 ` [PATCH v1 09/11] t: add tests for restore Nguyễn Thái Ngọc Duy
2019-03-09 16:53   ` Andrei Rybak
2019-03-13  9:13   ` Johannes Schindelin
2019-03-13  9:20     ` Duy Nguyen
2019-03-13 22:17       ` Johannes Schindelin
2019-03-14  4:57         ` Duy Nguyen
2019-03-14  5:45   ` Junio C Hamano
2019-03-14  5:55     ` Junio C Hamano
2019-03-08 10:16 ` [PATCH v1 10/11] completion: support restore Nguyễn Thái Ngọc Duy
2019-03-09 19:16   ` Elijah Newren
2019-03-11 15:22     ` Duy Nguyen
2019-03-11 15:39       ` Duy Nguyen
2019-03-11 18:28         ` Elijah Newren
2019-03-08 10:16 ` [PATCH v1 11/11] doc: promote "git restore" Nguyễn Thái Ngọc Duy
2019-03-09 19:37   ` Elijah Newren
2019-03-11 14:11     ` Randall S. Becker
2019-03-11 14:36       ` Duy Nguyen
2019-03-13  4:58         ` Junio C Hamano
2019-04-11 10:55           ` Duy Nguyen
2019-03-10 11:19 ` [PATCH v1 00/11] And new command "restore" Duy Nguyen
2019-03-10 22:45   ` Jacob Keller
2019-03-11 16:01   ` Elijah Newren
2019-03-13 22:58   ` Junio C Hamano
2019-03-14  3:49     ` Duy Nguyen
2019-04-11 13:12 ` [PATCH v2 00/16] Add new command 'restore' Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 01/16] checkout: split part of it to " Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 02/16] restore: take tree-ish from --source option instead Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 03/16] restore: make pathspec mandatory Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 04/16] restore: disable overlay mode by default Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 05/16] checkout: factor out worktree checkout code Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 06/16] restore: add --worktree and --staged Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 07/16] restore: reject invalid combinations with --staged Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 08/16] restore: default to --source=HEAD when only --staged is specified Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 09/16] restore: replace --force with --ignore-unmerged Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 10/16] restore: support --patch Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 11/16] t: add tests for restore Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 12/16] completion: support restore Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 13/16] user-manual.txt: prefer 'merge --abort' over 'reset --hard' Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 14/16] doc: promote "git restore" Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 15/16] rm: add --staged as alias for --cached Nguyễn Thái Ngọc Duy
2019-04-11 13:12   ` [PATCH v2 16/16] help: move git-diff and git-reset to different groups Nguyễn Thái Ngọc Duy
2019-04-12  5:14   ` [PATCH v2 00/16] Add new command 'restore' Junio C Hamano
2019-04-13 10:39     ` Duy Nguyen
2019-04-17 10:04   ` Duy Nguyen
2019-04-18  0:38     ` Junio C Hamano
2019-04-18  9:40       ` Duy Nguyen
2019-04-18 10:03       ` Johannes Schindelin
2019-04-18 10:20         ` Duy Nguyen
2019-04-25  9:45   ` [PATCH v3 " Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 01/16] checkout: split part of it to " Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 02/16] restore: take tree-ish from --source option instead Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 03/16] restore: make pathspec mandatory Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 04/16] restore: disable overlay mode by default Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 05/16] checkout: factor out worktree checkout code Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 06/16] restore: add --worktree and --staged Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 07/16] restore: reject invalid combinations with --staged Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 08/16] restore: default to --source=HEAD when only --staged is specified Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 09/16] restore: replace --force with --ignore-unmerged Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 10/16] restore: support --patch Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 11/16] t: add tests for restore Nguyễn Thái Ngọc Duy
2019-06-13 15:19       ` SZEDER Gábor
2019-04-25  9:45     ` [PATCH v3 12/16] completion: support restore Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 13/16] user-manual.txt: prefer 'merge --abort' over 'reset --hard' Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 14/16] doc: promote "git restore" Nguyễn Thái Ngọc Duy
2019-04-25  9:45     ` [PATCH v3 15/16] help: move git-diff and git-reset to different groups Nguyễn Thái Ngọc Duy
2019-04-25  9:46     ` [PATCH v3 16/16] Declare both git-switch and git-restore experimental Nguyễn Thái Ngọc Duy
2019-05-07  2:21     ` [PATCH v3 00/16] Add new command 'restore' Emily Shaffer
2019-05-07  4:57       ` Junio C Hamano
2019-05-07 10:36       ` Duy Nguyen
2019-05-07 18:31         ` Emily Shaffer
2019-05-08 10:20           ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BFtsQzDj3mCr1aRTpy2PQCzATjGGccECCmH8Qa_Y4aASQ@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=stefanbeller@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).