git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Duy Nguyen <pclouds@gmail.com>
Date
Mar 9, 2019, 12:16 UTC
Message-ID
<CACsJy8D4tvm_zLo0DcnjmcBeKDRDR+HGAxd1PsUMSBcgR59DPg@mail.gmail.com>
In-Reply-To
<CABPp-BFXZMorrHph3hGFnqfceHs68byWNgffNKGp1ov6X5-o5A@mail.gmail.com>
On Sat, Mar 9, 2019 at 1:01 AM Elijah Newren <newren@gmail.com> wrote:
Show 17 quoted lines
>
> 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...
>
> On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> > +SYNOPSIS
>
> It might be worth adding some words somewhere to differentiate between
> `reset` and `restore`.  E.g.
>
> `git restore` modifies the working tree (and maybe index) to change
> file content to match some other (usually older) version, but does not
> update your branch.  `git reset` is about modifying which commit your
> branch points to, meaning possibly removing and/or adding many commits
> to your branch.
>
> It may also make sense to add whatever description you use to the reset manpage.
Good point.
Show 8 quoted lines
> > +--------
> > +[verse]
> > +'git restore' [<options>] [--source=<revision>] <pathspec>...
> > +'git restore' (-p|--patch) [--source=<revision>] [<pathspec>...]
>
> So one cannot specify any special options with -p?  Does that mean one
> cannot use it with --index (i.e. this command cannot replace 'git
> reset -p')?  Or is this an oversight in the synopsis?
Oversight. -p can be used with either --index or --worktree or both.
Show 6 quoted lines
> > +
> > +When a `<revision>` is given, the paths that match the `<pathspec>` are
> > +updated both in the index and in the working tree.
>
> I thought the default was --worktree.  Is this sentence from an older
> version of your patch series that you forgot to update?
Oops.
Show 13 quoted lines
> > +-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.

Show 10 quoted lines
> > +-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.

Show 5 quoted lines
> Also, what happens when there are unmerged entries
> in the index and someone tries to restore just working tree files --
> are the ones corresponding to unmerged entries skipped (if so,
> silently or with warnings printed for the user?), or does something
> else happen?

If -m is also specified, then we recreate the conflict. The from code, if an unmerged path is skipped, there will be warnings.

Show 15 quoted lines
> > +
> > +-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).
>
> Should you mention that these are incompatible with --source and
> --index?  And perhaps also make sure the code aborts if either of
> these options are combined with either of those?

I will make sure that the code aborts. Not so sure about mentionging every incompatible combination though. Will it be too much? I think we catch and report plenty invalid combinations but I don't think we have mentioned them all (or maybe we have, I didn't check the document again)

Show 8 quoted lines
> > +       Just like linkgit:git-submodule[1], this will detach the
> > +       submodules HEAD.
> > +
> > +--overlay::
> > +--no-overlay::
> > +       In overlay mode, `git checkout` never removes files from the
>
> Why are you talking about `git checkout` here?  Shouldn't this be `git restore`?
Oops.
-- 
Duy
Previous: Elijah NewrenNext: Elijah Newren
Message 4 of 97 in “And new command "restore"”
  1. 00/11 And new command "restore"Nguyễn Thái Ngọc Duy, Mar 8, 2019
  2. 01/11 checkout: split part of it to new command 'restore'Nguyễn Thái Ngọc Duy, Mar 8, 2019
  3. Elijah NewrenMar 8, 2019
  4. Duy NguyenMar 9, 2019
  5. Elijah NewrenMar 9, 2019
  6. Duy NguyenApr 9, 2019
  7. Duy NguyenMar 25, 2019
  8. Elijah NewrenMar 25, 2019
  9. Johannes SchindelinMar 13, 2019
  10. 02/11 restore: take tree-ish from --source option insteadNguyễn Thái Ngọc Duy, Mar 8, 2019
  11. Elijah NewrenMar 9, 2019
  12. Eric SunshineMar 10, 2019
  13. 03/11 restore: make pathspec mandatoryNguyễn Thái Ngọc Duy, Mar 8, 2019
  14. Elijah NewrenMar 9, 2019
  15. 04/11 restore: disable overlay mode by defaultNguyễn Thái Ngọc Duy, Mar 8, 2019
  16. Elijah NewrenMar 9, 2019
  17. 05/11 checkout: factor out worktree checkout codeNguyễn Thái Ngọc Duy, Mar 8, 2019
  18. 06/11 restore: add --worktree and --indexNguyễn Thái Ngọc Duy, Mar 8, 2019
  19. Elijah NewrenMar 9, 2019
  20. Eric SunshineMar 10, 2019
  21. Duy NguyenMar 13, 2019
  22. Junio C HamanoMar 13, 2019
  23. 07/11 restore: default to --source=HEAD when only --index is specifiedNguyễn Thái Ngọc Duy, Mar 8, 2019
  24. Elijah NewrenMar 9, 2019
  25. 08/11 restore: support --patchNguyễn Thái Ngọc Duy, Mar 8, 2019
  26. Elijah NewrenMar 9, 2019
  27. 09/11 t: add tests for restoreNguyễn Thái Ngọc Duy, Mar 8, 2019
  28. Andrei RybakMar 9, 2019
  29. Johannes SchindelinMar 13, 2019
  30. Duy NguyenMar 13, 2019
  31. Johannes SchindelinMar 13, 2019
  32. Duy NguyenMar 14, 2019
  33. Junio C HamanoMar 14, 2019
  34. Junio C HamanoMar 14, 2019
  35. 10/11 completion: support restoreNguyễn Thái Ngọc Duy, Mar 8, 2019
  36. Elijah NewrenMar 9, 2019
  37. Duy NguyenMar 11, 2019
  38. Duy NguyenMar 11, 2019
  39. Elijah NewrenMar 11, 2019
  40. 11/11 doc: promote "git restore"Nguyễn Thái Ngọc Duy, Mar 8, 2019
  41. Elijah NewrenMar 9, 2019
  42. Randall S. BeckerMar 11, 2019
  43. Duy NguyenMar 11, 2019
  44. Junio C HamanoMar 13, 2019
  45. Duy NguyenApr 11, 2019
  46. Duy NguyenMar 10, 2019
  47. Jacob KellerMar 10, 2019
  48. Elijah NewrenMar 11, 2019
  49. Junio C HamanoMar 13, 2019
  50. Duy NguyenMar 14, 2019
  51. 00/16 Add new command 'restore'Nguyễn Thái Ngọc Duy, Apr 11, 2019
  52. 01/16 checkout: split part of it to new command 'restore'Nguyễn Thái Ngọc Duy, Apr 11, 2019
  53. 02/16 restore: take tree-ish from --source option insteadNguyễn Thái Ngọc Duy, Apr 11, 2019
  54. 03/16 restore: make pathspec mandatoryNguyễn Thái Ngọc Duy, Apr 11, 2019
  55. 04/16 restore: disable overlay mode by defaultNguyễn Thái Ngọc Duy, Apr 11, 2019
  56. 05/16 checkout: factor out worktree checkout codeNguyễn Thái Ngọc Duy, Apr 11, 2019
  57. 06/16 restore: add --worktree and --stagedNguyễn Thái Ngọc Duy, Apr 11, 2019
  58. 07/16 restore: reject invalid combinations with --stagedNguyễn Thái Ngọc Duy, Apr 11, 2019
  59. 08/16 restore: default to --source=HEAD when only --staged is specifiedNguyễn Thái Ngọc Duy, Apr 11, 2019
  60. 09/16 restore: replace --force with --ignore-unmergedNguyễn Thái Ngọc Duy, Apr 11, 2019
  61. 10/16 restore: support --patchNguyễn Thái Ngọc Duy, Apr 11, 2019
  62. 11/16 t: add tests for restoreNguyễn Thái Ngọc Duy, Apr 11, 2019
  63. 12/16 completion: support restoreNguyễn Thái Ngọc Duy, Apr 11, 2019
  64. 13/16 user-manual.txt: prefer 'merge --abort' over 'reset --hard'Nguyễn Thái Ngọc Duy, Apr 11, 2019
  65. 14/16 doc: promote "git restore"Nguyễn Thái Ngọc Duy, Apr 11, 2019
  66. 15/16 rm: add --staged as alias for --cachedNguyễn Thái Ngọc Duy, Apr 11, 2019
  67. 16/16 help: move git-diff and git-reset to different groupsNguyễn Thái Ngọc Duy, Apr 11, 2019
  68. Junio C HamanoApr 12, 2019
  69. Duy NguyenApr 13, 2019
  70. Duy NguyenApr 17, 2019
  71. Junio C HamanoApr 18, 2019
  72. Duy NguyenApr 18, 2019
  73. Johannes SchindelinApr 18, 2019
  74. Duy NguyenApr 18, 2019
  75. 00/16 Add new command 'restore'Nguyễn Thái Ngọc Duy, Apr 25, 2019
  76. 01/16 checkout: split part of it to new command 'restore'Nguyễn Thái Ngọc Duy, Apr 25, 2019
  77. 02/16 restore: take tree-ish from --source option insteadNguyễn Thái Ngọc Duy, Apr 25, 2019
  78. 03/16 restore: make pathspec mandatoryNguyễn Thái Ngọc Duy, Apr 25, 2019
  79. 04/16 restore: disable overlay mode by defaultNguyễn Thái Ngọc Duy, Apr 25, 2019
  80. 05/16 checkout: factor out worktree checkout codeNguyễn Thái Ngọc Duy, Apr 25, 2019
  81. 06/16 restore: add --worktree and --stagedNguyễn Thái Ngọc Duy, Apr 25, 2019
  82. 07/16 restore: reject invalid combinations with --stagedNguyễn Thái Ngọc Duy, Apr 25, 2019
  83. 08/16 restore: default to --source=HEAD when only --staged is specifiedNguyễn Thái Ngọc Duy, Apr 25, 2019
  84. 09/16 restore: replace --force with --ignore-unmergedNguyễn Thái Ngọc Duy, Apr 25, 2019
  85. 10/16 restore: support --patchNguyễn Thái Ngọc Duy, Apr 25, 2019
  86. 11/16 t: add tests for restoreNguyễn Thái Ngọc Duy, Apr 25, 2019
  87. SZEDER GáborJun 13, 2019
  88. 12/16 completion: support restoreNguyễn Thái Ngọc Duy, Apr 25, 2019
  89. 13/16 user-manual.txt: prefer 'merge --abort' over 'reset --hard'Nguyễn Thái Ngọc Duy, Apr 25, 2019
  90. 14/16 doc: promote "git restore"Nguyễn Thái Ngọc Duy, Apr 25, 2019
  91. 15/16 help: move git-diff and git-reset to different groupsNguyễn Thái Ngọc Duy, Apr 25, 2019
  92. 16/16 Declare both git-switch and git-restore experimentalNguyễn Thái Ngọc Duy, Apr 25, 2019
  93. Emily ShafferMay 7, 2019
  94. Junio C HamanoMay 7, 2019
  95. Duy NguyenMay 7, 2019
  96. Emily ShafferMay 7, 2019
  97. Duy NguyenMay 8, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.