Redo created file fails

[aeschli]

Testing #111015

• Create a new file in the explorer
• from the window menu, run Edit > Undo. File is gone ✔️
• from the window menu, run Edit > Redo. Nothing happens

[isidorn]

@aeschli I tried reproducing this and it requires some effort

1. Create a new file
2. Notice how the focus is automatically moved to editor as expected
3. Edit > Undo File is gone! The editor properly forwards the undo to the explorer (since focus is in Editor)
4. Edit > Redo. Nothing happens. The editor does not properly forward the redo to the explorer (since focus is still in Editor)

Step 3. is expected as we discussed #111659
Step 4. is not expected as it does not align with 3. thus aslo assigning this to @alexdima

[alexdima]

4. The editor does not properly forward the redo to the explorer (since focus is still in Editor)

The editor never forwards the redo nor undo to the explorer. The editor takes the current focused file, reads the URL from that and finds an undo/redo element with that URL in the undo/redo stack.

So if you create a file called A.txt and open it and focus it in the editor. If you run undo, the undo/redo elements for file A.txt are searched and executed. So an undo results in the deletion of the file, so the file is gone.

After this, if the focus is moved to a file B.txt that you had opened (in a background tab or something), the focus is in the file B.txt. So running redo will run redo any elements related to file B.txt.

I am not sure what to do here, this works exactly as implemented. @isidorn Please let me know what you would like to be done differently by the undo/redo service. So IMHO step 4 is 100% as designed. If at step 4 the focus goes to B.txt, the redo will redo in B.txt, not in A.txt.

By the way, I agree this is bad, but at this point I suggest we don't offer an undo of file creation. cc @jrieken for opinions.

[jrieken]

The editor never forwards the redo nor undo to the explorer.

Well, maybe that's what it should do? Like, when a UndoRedoSource is "empty" it could delegate to the "next" source.

[isidorn]

@alexdima thanks for clarifying.
I agree this is expected behavior.

I see how the undo can only be done from the explorer which does not have focus in this case.
I would not change anything here atm, even though @jrieken suggestions makes sense.

After we get more user feedback we might change if the explorer undo requires focus in the explorer...

[alexdima]

Well, maybe that's what it should do? Like, when a UndoRedoSource is "empty" it could delegate to the "next" source.

Yes, but the issue will remain and this might have unexpected consequences e.g. undoing in not visible files. e.g.

• create file A.txt
• open file B.txt, type in it
• open file C.txt, type in it
• open file A.txt
• press undo: file A.txt will be deleted, focus moves to C.txt
• press undo: typing in C.txt gets undo, focus remains in C.txt
• press undo: (new proposed logic kicks in and will undo an undo-redo element that is not associated with C.txt): typing in B.txt gets undone which is completely invisible because B.txt is behind C.txt

I think it is dangerous to undo undo-redo elements not associated with the current focused editor because edits occur which are invisible to the user.

---

I also agree that it is surprising that you create a file, type in it, hold cmd+z and the file disappears. I would expect that cmd+z stops when the file is empty. It feels to me that perhaps operations originating from the explorer should not be undoable from the editor... But that would ruin the java rename refactoring case, which would became undoable only when moving focus to the explorer. Thoughts?

[isidorn]

@alexdima I see the issue.

I was pondering about this a bit more and my feeling is:

• The current solution seems to be a bit confusing for the users (since all 3 testers filled some version of "Undo is not working because I have no clue where focus is").
• Changing the behavior that the Explorer Undoes without it having focus is dangerous as @alexdima also pointed out.
• Also as @alexdima pointed out: cmd+z from the Editor should not be able to undo operations originating from the explorer. That should at least make the current situaton less confusing - since currently some operations can be undoable and some can not

As I mentioned in other issues, I would probably not change anything here for Endgame, and in Debt week we can decide on a stragety.

My other concern is even if we change the third bullet point I think users will still be confused by undo not working in Explorer since the Explorer passes focus into the editor in some cases automatically.

[alexdima]

I agree with you @isidorn . My concern is that we might have to do a recovery build if many people are confused and we might have to live with this until end of January.

We could do the following: undoing from the editor will undo operations inserted by the explorer, but if it hits an undo-redo element inserted by the explorer, it could first prompt to confirm.

[isidorn]

That sounds like a good approach to me.

[alexdima]

This is what the prompt will read for this case:

[isidorn]

Looks good!

I can also fine tune the label from the Explorer.
I currently call it "New a.txt". I am open for suggestions
"Create a.txt"
"create a.txt"

[alexdima]

👍 I like "Create a.txt". I just pushed so you can test how it feels, running from source.

[isidorn]

Feels good to me, just tried it out.

[isidorn]

I like the flow, played around it a bit more...

Fyi testers for potential feedback @aeschli @meganrogge @rebornix

Gist of the above discussion: when there is an "aggresive" undo from the editor we now prompt.
What I mean by aggressive is editor undoing something which was intiated from the Explorer.
Also note that we distinguish undos based on where the focus is (as I mentioned in the test plan item)

[aeschli]

Maybe deleting a file (undoing a create, copy, move) should always prompt, regardless if invoked from the editor and explorer.

[isidorn]

@aeschli makes sense, however you can always redo to bring it back. So for now I would not prompt, but I am open to changing this.

[isidorn]

I beleive we are good here, since we now also introduced a prompt for destructive operations.
Undoing a create.

I am currently not prompting for undoing a move or copy - since you do not get data loss then. Though I am open to changing that.

Thus closing this