Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a git reset --hard {commit} equivalent #60

Closed
craigds opened this issue Apr 23, 2020 · 7 comments
Closed

Add a git reset --hard {commit} equivalent #60

craigds opened this issue Apr 23, 2020 · 7 comments

Comments

@craigds
Copy link
Member

craigds commented Apr 23, 2020

At present sno reset exists, but it actually does something totally unrelated to git reset: it throws away uncommited changes in your working tree. That's the equivalent of git checkout .

git reset actually doesn't touch your working tree; it modifies the repository head and possibly the index. So it's a totally different command.

In testing sno, I find myself needing a more gittish reset command so I can undo a commit easily. At present I'm using BRANCH=$(git symbolic-ref --short HEAD) ; sno checkout HEAD^ ; git branch -f $BRANCH HEAD ; sno checkout $BRANCH , but obviously that's cumbersome.

I assume a reset command would be useful generally, but at least for testing sno it's sorely missed.

@UberMouse
Copy link

UberMouse commented Apr 23, 2020

Will this let me peel a commit off so I can reset it? git reset HEAD~1 equivalent. Or only peel the commit off and discard the changes?

Edit: I totally thought this was a PR 😂

So yes, I agree. Being able to do a soft reset would be very helpful.

@olsen232
Copy link
Collaborator

FYI a slightly less magic / undocumented way - of resetting your branch head to a commit of your choice such as HEAD^ is as follows:

sno checkout HEAD^    # go back one commit
sno branch -D [branch_name]    # delete branch 
sno checkout -b [branch_name    # recreate branch at this point 

Of course we can make a reset command one day - seems like it has never quite been a priority - but at least this workaround is something I'd be okay with putting in public documentation.

@olsen232
Copy link
Collaborator

@rcoup I want Rob to okay this plan before I (or anyone) does any work on it - The way that sno reset does one simple thing at the moment might be an improvement on git's weird mixture of reset - hard, soft, and mixed - and checkouts for undoing things, and as such might be part of Rob's plan for sno to be simpler than git.

However, seems like a git reset --hard {commit} would still be useful, if we can work out where to put it.
Would it make sense if we just added an optional ref arguement to sno reset?
So:
sno reset is equivalent to sno checkout HEAD --discard-changes (as it is now)
sno reset REF would be equivalent to sno checkout REF --discard-changes, except that it also updates the current branch head to point to REF too. Or to put it another way, it would be the same as git reset --hard REF.

We could also add support for --hard and --soft, maybe, except - maybe that's adding too many confusing things again.

@craigds
Copy link
Member Author

craigds commented Jan 12, 2021

I find sno's reset command a bit confusing because it isn't the same as git's reset command at all:

  • git's reset command changes the repo head. optionally the index & working copy can be updated too (--hard)
  • sno's reset command changes only the working copy, and always resets to HEAD (there's no target argument at all). It is similar to git checkout -- .

IMO we shouldn't add a flag to the existing sno reset command to do something that git reset does - that'll only make it more confusing. Instead we should do one of the following:

  1. Rename sno reset to sno discard or similar, and then implement a reset command which is more compatible with git's reset command.
  2. Leave sno reset alone and implement a sno move-head command which does the same as git reset --hard. Naming this might need some work :/
  3. ... both 1 and 2? IMO it's confusing to have a reset that's completely different to git's one.

@rcoup
Copy link
Member

rcoup commented Jan 12, 2021

Reset, restore and revert

There are three commands with similar names: git reset, git restore and git revert.

  • git revert is about making a new commit that reverts the changes made by other commits.
  • git restore is about restoring files in the working tree from either the index or another commit. This command does not update your branch. The command can also be used to restore files in the index from another commit.
  • git reset is about updating your branch, moving the tip in order to add or remove commits from the branch. This operation changes the commit history. git reset can also be used to restore the index, overlapping with git restore.

So maybe we kinda follow that approach (except dropping the overlap between reset & restore?)

  • restore → manipulate working copy
  • reset → manipulate branch tip

And continue to prefer switch/restore instead of checkout for WC manipulations.

@olsen232
Copy link
Collaborator

olsen232 commented Jan 13, 2021

+1 to not using checkout for this stuff - git checkout does too many things:

- do nothing / populate the workdir if not yet populated (git checkout)
- change HEAD and update the entire workdir accordingly (git checkout COMMIT)
- update some subset of the workdir, don't update HEAD (git checkout [COMMIT] PATH)
- update the entire workdir, don't update HEAD (git checkout [COMMIT] .)
- some of the above and also create a new branch (-b)

(Some versions of this command are safe and warn me if they would overwrite any local changes - git checkout @^ - and other versions specifically overwrite local changes so don't warn me that they will - git checkout . - This stresses me out)

But that's an aside. The plan above is good as far as it goes:

  • restore → manipulate working copy
  • reset → manipulate branch tip

However, in Sno, so far we don't ever allow the branch tip to be manipulated without also resetting the working copy, unless the new branch tip is basically the same as the old one.

Here we differ from git. git switch BRANCH might keep local changes in some files even as it modifies other files, or it might warn the user "Your local changes to the following files would be overwritten by checkout".
Whereas, sno switch BRANCH will either keep local changes because it the other BRANCH is basically the same as this branch, or it will warn: You have uncommitted changes in your working copy. Commit these changes first (sno commit) or just discard them by adding the option --discard-changes.

We could change this behaviour to be more like git if it seems important - we can't be exactly like git since, for instance, a newly added file can almost always be kept when changing branches in git, but a newly inserted feature maybe can't be kept when changing branch in sno - specifically, it can't be kept if it doesn't match the target branch schema.

Assuming we don't change this behavior, this would mean that most resets would also be restores as well. Or to put it differently, most resets would be hard resets. Here is the full list of possibilities, with behaviour consistent with existing Sno commands (eg switch / checkout):

  • sno restore → revert all working copy edits, set working copy state to match HEAD exactly
  • sno reset COMMIT → set branch tip* to be COMMIT, as long as there are no working copy edits. Otherwise fail with error message.
  • sno reset COMMIT --discard-changes → set branch tip* to be COMMIT, and set working copy state to match COMMIT exactly.

What do you think - is this good enough? Or do we need to add functionality more in line with what git can do - switch / reset operations that change some parts of the working copy, while keeping edits to other parts of the working copy?

*If HEAD is detached and there is no branch tip, reset affects HEAD directly. In this case reset works the same as checkout.

@rcoup
Copy link
Member

rcoup commented Jan 13, 2021

What you've proposed sounds good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants