atom / github

:octocat: Git and GitHub integration for Atom
https://github.atom.io
MIT License
1.11k stars 392 forks source link

New undo last commit/amend breaks workflow #1466

Open ghost opened 6 years ago

ghost commented 6 years ago

Description

The changes introduced to undoing a commit/amending to a commit with #1364 breaks a lot of workflows.

Until 1.26. it was possible to do the following things, which are now impossible:

Instead now all I have is an amend without any preview, and an undo which irreversibly rewrites history as soon as I click it.

This was a great tool, that can't be reproduces with normal Git and that is now gone.

Please either bring the checkbox back or give me the ability to bring it back. The new version is a massive step backwards functionally in my eyes.

Old amend checkbox

Old amend checkbox

Versions

1.27.0

MarionCrp commented 6 years ago

I agree ! Loved the amend commit functionality ! (what about a "enabled" checkbox in settings ? )

DzmitryBiruk commented 6 years ago

Yeah i agree, This changes broke my workflow

gjtorikian commented 6 years ago

I also loved the amend checkbox. Where did it go?

smashwilson commented 6 years ago

One of our design priorities with this package is to maintain fidelity with the reality of git's state on disk. There are a lot of advantages to doing this: you can change context seamlessly between Atom and any other git tool, including the command line, even mid-operation; the diff markers in the gutters are accurate; you don't lose any work if you close and re-open Atom; and so on.

The previous behavior of the amend checkbox was one of the only places in the package where this was not the case. It caused us a lot of headaches and was the root cause of some fairly subtle bugs. After quite a bit of consideration, we replaced it with the implementation we have now, which does maintain that fidelity. "Undo" is actually a bit more powerful now, too: you can undo several commits in sequence and re-commit them in different chunks, like a rebase-lite.

Easily see the changes from the last commit without having to undo

With a clean index, undo and commit are symmetric options; if you click undo and commit without changing anything, the HEAD commit is the same... with the exception of its timestamp, so the commit SHA does change.

Please either bring the checkbox back or give me the ability to bring it back. The new version is a massive step backwards functionally in my eyes.

One of the downsides of making design changes in a product with as many users as Atom is that we're going to make someone upset as we iterate. It won't be the last time, and I for one appreciate the level of diplomacy on display here so far :wink:

Sharing concrete use-cases that aren't supported is helpful - it gives us more data to see how these features are actually being used, and either come up with solutions that help you accomplish the same tasks in a new way, or consciously take them out of scope and decide not to support them. (We can't support everything after all!)

gjtorikian commented 6 years ago

I must be going blind, actually -- I didn't even see that undo button until you mentioned it. For me, personally, 99% of my use cases are just adding one more file / hunk to a previous commit -- and the last 1% is rewriting a mistyped commit message.

So yeah, the undo button works for me. ☺️

essenmitsosse commented 6 years ago

Thanks @smashwilson for the reply and clarification. I definitely understand your reasoning. Lets just refer to that XKCD: https://xkcd.com/1172/

Being able to get back the commit that was just undone would solve 90% of the issue for me. How about adding a button, that does something like git reset HEAD@{1}. Of course it would be awesome if Atom would be able to decide if a new commit is necessary or if it can just restore the old one. But for the time being just being able to quickly redo the undo from the UI would make the workflow possibilities far more what they used to be.

This basically means:

Old New
Tick amend checkbox Undo Button
Untick amend checkbox Button/context menu that does something like git reset HEAD@{1}
simurai commented 6 years ago

@gjtorikian For me, personally, 99% of my use cases are just adding one more file / hunk to a previous commit

For that use case, there is now an "Amend" option in the context menu when right-clicking the last commit.

Amend Undo
—————————————————————— ——————————————————————
I forgot to add something to my last commit I want to "edit" my last commit (add/remove changes, rewrite commit message, add/remove co-authors)
Right-click -> Amend Click "Undo" button
amend undo
gjtorikian commented 6 years ago

Whoa, even better!

essenmitsosse commented 6 years ago

@simurai I was aware of that feature but I admittedly misphrased that a bit.

The amend feature doesn’t solve the initial problem: being able to see the last commit or the result of a potential amend, while still being able to get back to the original commit without history rewrite in case no amend turns out to be necessary.

what is needed is the ability to redo an undo so i can securely move the last commit into the staging area.

simurai commented 6 years ago

see the result of a potential amend

Yeah, I can see that being useful. 👍 When clicking on "Undo", it might be hard to manually untangle the "amended" changes again.

what is needed is the ability to redo an undo so i can securely move the last commit into the staging area.

One way could be to keep the last commit, but have it look disabled and add a button that says [Redo undo]:

image

But that looks quite confusing. :laughing: And might bring back the "subtle bugs" mentioned above ☝️

Something we could consider for the future: Be able to see the preview of an "Amend". But first we would need some sort of multi-file diff.

Or be able to squash two commits with a preview. And other options like reorder commits or edit the commit message. Then you don't have to worry too much about the history while you make commits and can do it later before you push.

essenmitsosse commented 6 years ago

Something we could consider for the future: Be able to see the preview of an "Amend". But first we would need some sort of multi-file diff.

That`s basically what the old "amend" checkbox used to do.

But yeah, that redo undo button would be great.

And just using git reflog internally should make the implementation native to git und thus hopefully avoid any subtle bugs.

gliviu commented 6 years ago

I just lost some important changes when I had some files staged and accidentally pressed the undo button. This new feature is a complete hazard.
Not to mention it is totally counter intuitive. I'm working with git for a couple of years now, still it took me quite some experiments to understand how the new proposal is supposed to work. My opinion - this should definitely be rolled back.

mpacer commented 6 years ago

I just want to chime in that this massively breaks my workflow. This was one of my favorite features of Atom's git integration — I didn't know it if you had asked though. It was so smooth a feature that I didn't even realise how great it was. But now… I'm hobbled.

I frequently use it for both use cases: editing the commit message and adding/removing changes after a bit more of development time.

But in particular it is great because I end up toggling back and forth frequently when I'm trying to decide whether things should be in their own commits or joined with others. It allowed me to not need to make a decision while still seeing what things would look like without triggering any githooks (which can sometime get pretty weighty and slow).

I appreciate the considerations regarding the symmetry of the operation and its fidelity to git (not to mention subtle bugs). But, one of the greatest aspects of a UI built on top of git is the way that it can expose functionality that would be hard to express in the original set of operations that dramatically ease the use of a tool known for its trickiness.

I know it's unlikely, but please consider adding the toggle back as a configurable option that is off by default but still available for those who rely on it.

jrial commented 6 years ago

@smashwilson Tx for the clarification.

As others have already mentioned: the two common use cases for the "amend" checkbox were:

  1. Quickly add a small chunk that was forgotten
  2. Reword the last commit message

Both of these can be achieved with the new "undo" button, but "undo" is an ambiguous, and therefore scary word. Especially in the context of a tool that has a gazilion barrels, all aimed squarely at your foot (git, not Atom). How about "Soft reset", "Uncommit" or "Restage" instead? Additionally, a tooltip could be added to the button that clarifies "This undoes the last commit, but keeps its changes in the staging area".

Anyway, some remarks on some of your other points:

With a clean index, undo and commit are symmetric options; if you click undo and commit without changing anything, the HEAD commit is the same... with the exception of its timestamp, so the commit SHA does change.

Then it's not a symmetric option. But it can be made symmetric by adding a "redo" or "recommit" button, which others have also brought up. This button would check out the commit again, unmodified, whilst preserving the other changes on the staging area. I think this is an elegant solution to the problem brought up by the OP:

I would like to provide a better, first-class way to view the content of a previous commit. More thoughts on that later.

I suggest showing the standard diff view currently used for preparing the commit, upon selecting the commit in question. But if you're thinking something more powerful, by all means. :)

It might also be interesting to preserve the commit's timestamp if you don't modify anything else, so the SHA wouldn't change if you undo and immediately re-commit. We'll have to think about that one.

If nothing has changed, the commit button could simply check out the commit SHA rather than performing a new commit. But this is an optimisation that is entirely optional IMHO; if we have a "redo" button, there's little need in complicating the functionality of the "commit" button.

Another thought is to visually distinguish between commits that have and have not been pushed yet, because commits that have been pushed are riskier to undo.

Please also add a popup, warning of the dangers of doing so, with a "don't show again" tickbox for those of us who know what they're doing.

bbugh commented 6 years ago

I'm echoing everyone else saying this really crippled my workflow. In addition to the trouble removing this feature has caused, atom-github was one of the only tools that supports worktrees properly, and so now I'm actually really crippled because I don't know of any other GUI tools that allow me to quickly amend a commit for all of the use cases above. The GUI is really helpful for patch amends, which the command line sucks at.

In the mean time, is there some way to go back to a previous atom-github version?

danhab99 commented 6 years ago

I don't see what was the point of cutting the tickbox. It worked fine. This solution just hides the amend feature.

kubenstein commented 6 years ago

@bbugh the Amend checkbox was removed in v1.27.0 (2018-05-15) so every version prior will still have the feature. Last one with the checkbox is v1.26.1 https://github.com/atom/atom/releases/tag/v1.26.1

changelog: https://atom.io/releases

nebularg commented 6 years ago

My issue with using undo vs being able to amend directly is that it destroys the commit metadata, ie, it's like doing git commit --amend --reset-author. The amend context menu option is fine for adding things, but you can't remove like you could with the checkbox (or git-gui's amend toggle).

jrial commented 6 years ago

@nebularg I don't quite follow. You can still remove by reverting part of the file back, staging that code change, and amending. The way you would do on the commandline.

But I do agree that it was easier when the tickbox was still there, as it allowed you to see the code changes from the to-be-amended commit in isolation. That workflow was a powerful addition, and achieving the same has now become a lot clumsier.

nebularg commented 6 years ago

@jrial that bit was just a "it breaks my workflow :confounded:" comment on how it use to behave the same as git-gui, not that it wasn't possible to get the same result.

HebaruSan commented 6 years ago

This change has also hurt discoverability. I got used to the checkbox, and when I updated to the current version it looked like this package had dropped support for amending altogether. I did eventually guess that right clicking the commit list might do something, but that's a pretty unfriendly place to hide an important action.

ngrilly commented 6 years ago

I fully agree with @mpacer. The amend checkbox was great. I was very happy discovering this feature in Atom, because I also use gitx which does something very similar. I understand the rationale explained by @smashwilson, but I still think the new design does not fully cover the workflow covered by the old design.

ngrilly commented 6 years ago

@bbugh wrote:

I don't know of any other GUI tools that allow me to quickly amend a commit for all of the use cases above

gitx does this, but it's another tool. Having this built-in in Atom was great.

ngrilly commented 6 years ago

For future reference, here is the issue that led to the design of the "amend commit" feature, that was recently replaced by the new "undo commit" feature: https://github.com/atom/github/issues/256.

After having read this, I have to agree with @smashwilson that the old design had some issues, which are mentioned in the original discussion.

I note that VSCode is worse, because their "Undo last commit" resets the working index (the currently staged changes) and writes the "last commit" which is undone to the working tree, which is really bad if you have ongoing changes in your working tree.

knoopx commented 5 years ago

After going through all the discussion I still don't understand why it was removed and how "Undo" super-seeds the previous behaviour. The "tick to amend" is a super-powerful way to interactively stage or discard previously committed chunks, it doesn't matter if its because you forgot something or because you staged something by accident or because you want to branch out with/without some specific code or you rebased and need to update code.

I agree it is way more complex as you need to keep a transitioning state when "amending" and the UI needs to properly handle it but I don't understand the reasoning behind the "persisted state" and "consistency" arguments.

When "amending" your are in a transitioning state so other apps don't need to know what is going on until you actually commit the changes. It is just the way it is displayed in the UI that is different and the way "patches" are generated and staged into the staging tree when staging or discarding chunks.

The current implementation might be simpler and pure but it is not quicker or friendlier than a git reset HEAD^ / git commit --amend while the "tick to amend" really improves the UX of something not possible/easy from the command line.

The way the git panel is right now is useless to me as it offers no improvement over the command line. Just my 2 cents.

domq commented 5 years ago

Between the three editors that I use (Emacs and the IntelliJ suite being the other two) Atom is the only one where amending a commit is so convoluted an operation. I had to read through this issue to get the hint for that "undo" button thing. At the very least the choice of a label is unfortunate. I think splitting the Commit button with a small down arrow to the right that pops up a context menu, would be more discoverable.

nycki93 commented 4 years ago

This is a footgun and should not be enabled by default. I use Git because I trust that "everything is safe in the version history", and this breaks that trust. I clicked undo on a commit, expecting it to just checkout the previous revision, or maybe do a revert, but it actually rewrote my history and I lost the commit completely! Thinking maybe I could fix the damage, I clicked the button again, hoping to "undo the undo", and it just got worse!!

I'm lucky that commit existed elsewhere and I could pull it back down -- if I didn't have a backup, this would have completely screwed me. There is an expectation in Git that you can undo everything, unless you go out of your way to change history with a squash or a filter-branch. You shouldn't be able to rewrite history by accident, especially not by default, and especially with as innocuous a name as "undo". If nothing else, I'd like to be able to disable this feature so I can't accidentally shoot myself in the foot with it again.

jrial commented 4 years ago

I'm lucky that commit existed elsewhere and I could pull it back down -- if I didn't have a backup, this would have completely screwed me.

Pro tip: familiarise yourself with the git reflog. It can save your ass in similar circumstances where the commit does not exist elsewhere.

There is an expectation in Git that you can undo everything, unless you go out of your way to change history with a squash or a filter-branch.

And undo does exactly what it promises: it "undoes" a commit. Which means it makes it so that it would appear as if it never happened. Which means rewriting history. The expectation is therefore not violated.

You just made the wrong assumption ("undo" means "revert" or "check out previous commit") based on a flawed assumption (external tools should not use "dangerous" Git operations). Furthermore, you failed to create a scratch branch to test your assumption on, which is even more important when you lack the knowledge of how to recover from a screwup, as you do. How exactly did you arrive at the conclusion that the fault for the mess you found yourself in lies with Atom?

the-woocash commented 3 years ago

This is a footgun and should not be enabled by default. I use Git because I trust that "everything is safe in the version history", and this breaks that trust. I clicked undo on a commit, expecting it to just checkout the previous revision, or maybe do a revert, but it actually rewrote my history and I lost the commit completely! Thinking maybe I could fix the damage, I clicked the button again, hoping to "undo the undo", and it just got worse!!

THIS.

It's crazy. If you delete a file in most GUIs you'll get a confirmation popup. You are used to having recycle bin for your files if you mess up. Two recycle bins in Your email account (Exchange). You are used to working with git, so that your work is safe and there would be nothing that you can click in any app's GUI that will irreversibly delete your work without warning you beforehand, without being marked with bold red letters with something like "remove", "delete", "erase".

Yet here You can click an "Undo" button, that reverts a change that you mentally know is safely stored in a repo. The button has innocuous colour and no confirmation upon pressing it. When you press it, some windows above fill with data, and the undo button "moves" to a previous commit, luring you, wanting you to press it again. And in this config if you do, your files are gone.

Your files are permanently gone after pressing Undo twice. Wow. Good we do backups. Still crazy.