Message ID | 20190425094600.15673-1-pclouds@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | Add new command 'restore' | expand |
Hiya, On Thu, Apr 25, 2019 at 04:45:44PM +0700, Nguyễn Thái Ngọc Duy wrote: > v3 changes are really small > > - gitcli.txt is updated to mention the --staged/--worktree pair, in > comparison to --cached/--index which is also mention there. > > - The patch 'rm --staged' is dropped. It could come back. But if it > does, it should be in a series where commands that take > --cached/--index will now take both --staged/--worktree. Those are > 'rm', 'apply', 'check-attr', 'grep', 'diff' and maybe 'ls-files'. > > In other words we support --staged/--worktree everywhere while > --cached/--index still remains because of muscle memory. This is > of course only a good move if --staged/--worktree is much better > than --cached/--index. > > - Since there's a chance this series may end up in 'master' and get > released, and I'm still worried that I screwed up somewhere, the last > patch declares the two commands experimental for maybe one or two > release cycles. > > If the reception is good, we revert that patch. If something > horrible is found, we can still change the default behavior without > anybody yelling at us. Or worst case scenario, we remove both > commands and declare a failed experiment. > > PS. the intent-to-add support is still not in. But it should be in > before the experimental status is removed. I've got a usability comment, as we're giving restore a try within Google for now. I found myself in a situation where I had accidentally staged all my changes to tracked files (I think resulting from a stash pop which generated a merge conflict?) and didn't see a good way to unstage everything using restore. I tried out `git restore --staged *` and it tried to restore every build artifact in my working tree, all of which should be ignored, made a lot of noisy errors, and left me with my changes still staged. Then, figuring that I only had a few files, I thought I'd type them each, with the exception of a .c/.h pair: g restore --staged builtin/log.c builtin/walken.c revision.* Because I had build artifacts present, this vomited while trying to unstage revision.o, and again left me with all my changes still staged. revision.o is validly ignored: $ g check-ignore revision.o revision.o Finally explicitly naming each file which I needed to restore worked, but I have very little interest in doing this for more than a handful of files, especially since the output of `git status` is not easy to paste into the command line (due to the "modified:" etc marker). I was definitely still able to make it do what I wanted with `git reset HEAD` but thought you may be interested. It'd be cool to have a hint which advises how you can unstage everything, or else to pay attention to the gitignore and not try to unstage those files, or else to have a command to unstage everything. (I looked for one by trying `git restore --staged -a` but that doesn't exist :) ) Thanks! Emily
Emily Shaffer <emilyshaffer@google.com> writes: > Then, figuring that I only had a few files, I thought I'd type them > each, with the exception of a .c/.h pair: > > g restore --staged builtin/log.c builtin/walken.c revision.* > > Because I had build artifacts present, this vomited while trying to > unstage revision.o, and again left me with all my changes still staged. > revision.o is validly ignored: > > $ g check-ignore revision.o > revision.o Often a claim "validly ignored" by users needs to be taken with a grain of salt. When it is in the index, it is no longer ignored. Assuming that you do not have revision.o in the index, and if "restore" is still "checkout -- <pathspec>" in disguise, then I think you could ask "git" not your shell to expand the asterisk, by protecting it from the shell, i.e. $ git restore --staged revision.\* and the pathspec should not match untracked revision.o. And if that does not work, you found a bug in restore, I think ;-).
On Tue, May 7, 2019 at 9:21 AM Emily Shaffer <emilyshaffer@google.com> wrote: > I've got a usability comment, as we're giving restore a try within > Google for now. Thanks. I thought I was the only guinea pig :D and obviously not a good one since I know too much (which is not a good thing as a tester). > I found myself in a situation where I had accidentally > staged all my changes to tracked files (I think resulting from a stash > pop which generated a merge conflict?) and didn't see a good way to > unstage everything using restore. > > I tried out `git restore --staged *` and it tried to restore every build > artifact in my working tree, all of which should be ignored, made a lot of > noisy errors, and left me with my changes still staged. For the record, "git restore --staged :/" should do the trick and it is documented as an example (but without --staged). Either way. I think you raise a good point about "*" (or patterns matching more than expected in general). I need to sleep on it and see if the old way of handling pattern matching failure is still a good way to go. > Finally explicitly naming each file which I needed to restore worked, > but I have very little interest in doing this for more than a handful of > files, especially since the output of `git status` is not easy to paste > into the command line (due to the "modified:" etc marker). For just a couple files, you could also use "-p" to go through them and either "a" to restore all or "d" to skip the file. Maybe adding "-i/--interactive" to git-restore too, just like "git add". Hmm...
On Tue, May 07, 2019 at 05:36:56PM +0700, Duy Nguyen wrote: > On Tue, May 7, 2019 at 9:21 AM Emily Shaffer <emilyshaffer@google.com> wrote: > > I've got a usability comment, as we're giving restore a try within > > Google for now. > > Thanks. I thought I was the only guinea pig :D and obviously not a > good one since I know too much (which is not a good thing as a > tester). > > > I found myself in a situation where I had accidentally > > staged all my changes to tracked files (I think resulting from a stash > > pop which generated a merge conflict?) and didn't see a good way to > > unstage everything using restore. > > > > I tried out `git restore --staged *` and it tried to restore every build > > artifact in my working tree, all of which should be ignored, made a lot of > > noisy errors, and left me with my changes still staged. > > For the record, "git restore --staged :/" should do the trick and it > is documented as an example (but without --staged). Yeah, this worked, and today I also noted `git restore --staged .` works, as does Junio's suggestion on the other mail (`git restore --staged revision.\*`), and quoting the * (`git restore --staged '*'`). So maybe I didn't think outside the box enough before mailing :) > > Either way. I think you raise a good point about "*" (or patterns > matching more than expected in general). I need to sleep on it and see > if the old way of handling pattern matching failure is still a good > way to go. I think it's worth considering, especially as `git reset HEAD *` and `git reset HEAD` both work. (`git restore --staged` complains that it hasn't got any paths, but reset seems to figure you just mean everything.) It was a surprising failure to me coming away from years of using reset. > > > Finally explicitly naming each file which I needed to restore worked, > > but I have very little interest in doing this for more than a handful of > > files, especially since the output of `git status` is not easy to paste > > into the command line (due to the "modified:" etc marker). > > For just a couple files, you could also use "-p" to go through them > and either "a" to restore all or "d" to skip the file. Maybe adding > "-i/--interactive" to git-restore too, just like "git add". Hmm... > -- > Duyh Anyways, maybe it's not a particularly weighty complaint, since there are still ways to mass unstage, and if I had just RTFM I would have found them. :) It's a new workflow, after all, and the old one still works, so it's probably not reasonable to expect it all to just work the same way with s/reset HEAD/restore --staged/. Thanks! - Emily
On Wed, May 8, 2019 at 1:31 AM Emily Shaffer <emilyshaffer@google.com> wrote: > > > I found myself in a situation where I had accidentally > > > staged all my changes to tracked files (I think resulting from a stash > > > pop which generated a merge conflict?) and didn't see a good way to > > > unstage everything using restore. > > > > > > I tried out `git restore --staged *` and it tried to restore every build > > > artifact in my working tree, all of which should be ignored, made a lot of > > > noisy errors, and left me with my changes still staged. > > > > For the record, "git restore --staged :/" should do the trick and it > > is documented as an example (but without --staged). > > Yeah, this worked, and today I also noted `git restore --staged .` > works, as does Junio's suggestion on the other mail (`git restore > --staged revision.\*`), and quoting the * (`git restore --staged '*'`). > So maybe I didn't think outside the box enough before mailing :) No it's good. It actually got me thinking. Actually I take that back. It's bad, I'm quite stuck at thinking here :D > > Either way. I think you raise a good point about "*" (or patterns > > matching more than expected in general). I need to sleep on it and see > > if the old way of handling pattern matching failure is still a good > > way to go. > > I think it's worth considering, especially as `git reset HEAD *` and > `git reset HEAD` both work. (`git restore --staged` complains that it > hasn't got any paths, but reset seems to figure you just mean > everything.) It was a surprising failure to me coming away from years of > using reset. Yeah I agree that doing it the "git reset" way makes sense for --staged because shell expansion is tied to worktree files, and --staged is completely disconnected from worktree. Shell expansion cannot get this case right, no point punishing the user because of it. My problem is "what about the other two cases, --worktree alone or --worktree with --staged (iow do we care about consistency)? what about git-checkout? does the old way even make sense for git-checkout? any better way to do if the reason behind is still valid?". I obviously haven't worked it all out yet.