| Message ID | 20190308101655.9767-11-pclouds@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | And new command "restore" | expand |
On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > > Completion for restore is straightforward. We could still do better > though by give the list of just tracked files instead of all present > ones. But let's leave it for later. s/give/giving/ I'm slightly worried that due to using --no-overlay mode by default in restore, having tab-completion include untracked files increases the risk of accidentally nuking the wrong file. restore is a destructive command anyway and should thus be used with care, so perhaps this isn't a big deal, but I thought I'd mention it.
On Sun, Mar 10, 2019 at 2:17 AM Elijah Newren <newren@gmail.com> wrote: > > On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > > > > Completion for restore is straightforward. We could still do better > > though by give the list of just tracked files instead of all present > > ones. But let's leave it for later. > > s/give/giving/ > > I'm slightly worried that due to using --no-overlay mode by default in > restore, having tab-completion include untracked files increases the > risk of accidentally nuking the wrong file. restore is a destructive > command anyway and should thus be used with care, so perhaps this > isn't a big deal, but I thought I'd mention it. This makes me think about my "git restore :/" example, which does not remove untracked files because its source is the index. But isn't it inconsistent that we only remove untracked files with --source and keep them without? Will that just confuse people more? Or did I just forget to implement no-overlay mode for the index too? Side note, to enable overlay mode again in git restore, maybe "git restore --keep-untracked --source..." will be better than "git restore --overlay --source..."
On Mon, Mar 11, 2019 at 10:22 PM Duy Nguyen <pclouds@gmail.com> wrote: > > On Sun, Mar 10, 2019 at 2:17 AM Elijah Newren <newren@gmail.com> wrote: > > > > On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > > > > > > Completion for restore is straightforward. We could still do better > > > though by give the list of just tracked files instead of all present > > > ones. But let's leave it for later. > > > > s/give/giving/ > > > > I'm slightly worried that due to using --no-overlay mode by default in > > restore, having tab-completion include untracked files increases the > > risk of accidentally nuking the wrong file. restore is a destructive > > command anyway and should thus be used with care, so perhaps this > > isn't a big deal, but I thought I'd mention it. > > This makes me think about my "git restore :/" example, which does not > remove untracked files because its source is the index. But isn't it > inconsistent that we only remove untracked files with --source and > keep them without? Will that just confuse people more? Or did I just > forget to implement no-overlay mode for the index too? Nope you confused me. non-overlay mode never touches untracked files and so neither does git-restore. It does make the description of git-restore about "remove if paths do not exist" incorrect. But frankly I find it hard to explain non-overlay mode with the index being the remove filter. In git-checkout, where we update both index and worktree, this may make sense to use the index as the remove filter. But worktree works on the worktree only by default. I'll need to sleep on this.
// CC'ing Thomas since this touches on a thread elsewhere about the glossary On Mon, Mar 11, 2019 at 8:40 AM Duy Nguyen <pclouds@gmail.com> wrote: > > On Mon, Mar 11, 2019 at 10:22 PM Duy Nguyen <pclouds@gmail.com> wrote: > > > > On Sun, Mar 10, 2019 at 2:17 AM Elijah Newren <newren@gmail.com> wrote: > > > > > > On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > > > > > > > > Completion for restore is straightforward. We could still do better > > > > though by give the list of just tracked files instead of all present > > > > ones. But let's leave it for later. > > > > > > s/give/giving/ > > > > > > I'm slightly worried that due to using --no-overlay mode by default in > > > restore, having tab-completion include untracked files increases the > > > risk of accidentally nuking the wrong file. restore is a destructive > > > command anyway and should thus be used with care, so perhaps this > > > isn't a big deal, but I thought I'd mention it. > > > > This makes me think about my "git restore :/" example, which does not > > remove untracked files because its source is the index. But isn't it > > inconsistent that we only remove untracked files with --source and > > keep them without? Will that just confuse people more? Or did I just > > forget to implement no-overlay mode for the index too? > > Nope you confused me. non-overlay mode never touches untracked files > and so neither does git-restore. > > It does make the description of git-restore about "remove if paths do > not exist" incorrect. But frankly I find it hard to explain > non-overlay mode with the index being the remove filter. In > git-checkout, where we update both index and worktree, this may make > sense to use the index as the remove filter. But worktree works on the > worktree only by default. I'll need to sleep on this. So, I guess this means that my addendum to Thomas' glossary entry for overlay is wrong; instead of: [[def_overlay]]overlay:: Only update and add files to the working directory, but don't delete them, similar to how 'cp -R' works. This is the default in a <<def_checkout,checkout>>. In contrast, no-overlay mode will also delete files not present in the source, similar to 'rsync --delete'. the definition should be (note the insertion of 'tracked'): [[def_overlay]]overlay:: Only update and add files to the working directory, but don't delete them, similar to how 'cp -R' works. This is the default in a <<def_checkout,checkout>>. In contrast, no-overlay mode will also delete tracked files not present in the source, similar to 'rsync --delete'. ...and perhaps the reason this makes it hard for you to explain when the index is the source is because this definition means there is no difference between overlay and non-overlay mode for that specific case. Am I on the right page now?
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 7fcf28d437..0b22e68187 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2483,6 +2483,21 @@ _git_reset () __git_complete_refs } +_git_restore () +{ + case "$cur" in + --conflict=*) + __gitcomp "diff3 merge" "" "${cur##--conflict=}" + ;; + --source=*) + __git_complete_refs --cur="${cur##--source=}" + ;; + --*) + __gitcomp_builtin restore + ;; + esac +} + __git_revert_inprogress_options="--continue --quit --abort" _git_revert ()
Completion for restore is straightforward. We could still do better though by give the list of just tracked files instead of all present ones. But let's leave it for later. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- contrib/completion/git-completion.bash | 15 +++++++++++++++ 1 file changed, 15 insertions(+)