Undo of rename Java file has no effect

[egamma]

Testing #107617

• Remote-Containers: Try a Sample
• Select Java
• Open AppTest.java
• Rename the file name of AppTest.java to AppTest2.java -> the class name is also changed to AppTest2
• undo -> the class name is changed back to AppTest but the file name is not renamed to AppTest.java

[alexdima]

@egamma

When you say:

Rename the file name of AppTest.java to AppTest2.java -> the class name is also changed to AppTest2

How do you rename the file name? Is there a code action that you do or do you do rename from the explorer?

AFAIK the file operations done from the explorer are not undoable (they don't push elements to the undo/redo service).

[egamma]

How do you rename the file name? Is there a code action that you do or do you do rename from the explorer?

The rename is done from the explorer and this triggers a refactoring.

[isidorn]

Closing as dup of #9390
This is a fair feature request which users want and that we can plan for.

[isidorn]

Reopening this issue, because it is more scoped and is a good first step for investigation of undo / redo in Explorer

[jrieken]

I know that @bpasero investigated how to use the BulkEditService in the tree - instead of the working copy service. Now sure how far he got

[isidorn]

@jrieken thanks. I'll sync up with @bpasero

[bpasero]

Yeah did not get to actually implementing this, but using bulk edit service over working copy service in explorer makes sense to me.

[alexdima]

👍 It might be good to start only with the case of rename file, because, for example, AFAIK if you create a delete file workspace edit, it will end up reading the file from disk and storing it in memory in order to be able to undo the deletion. Also LMK if you run into undo/redo problems.

[isidorn]

Thanks @alexdima for the heads up.
I started looking into this, by using the bulk edit service for all operations in the Explorer just to see how far I get and it seems to mostly work fine.
There's missing support for some operations, but I was talking to @jrieken about that

As for storing a file in memory, I guess that is not fine for large files, so could we simply have some limit on the service side?
I see a slight undo/redo issue, but I will file a seperate issue for that soon. Thanks!

We can discuss more in the standup also.

[isidorn]

Thanks to @jrieken this now works, thus closing.
@testforstephen if you have time to try this out from the Java side.

[testforstephen]

Tried in 2020-11-19 insider version, renaming to refactor a Java file from file explorer, undo only roll back the file contents, but not restore the file name.

[isidorn]

@testforstephen I apologize, this should hopefully work with today's insiders from 11-20.
@jrieken are there some changes needed on the Java side?

[testforstephen]

i tried the insider 11-23 again, here is the result.

• Rename AppTest.java to AppTest1.java from file explorer. -> The class name in the file is changed to AppTest1.
• undo -> The class name is changed back to AppTest, but the file name remains the same.
• undo again -> The file name is changed back to AppTest.java.

It seems that @jrieken's change is for the onWill-file event, but Java extension performs rename refactoring inside the onDidRenameFiles event. We chose the onDidRenameFiles event because the onWillRenameFiles event only gives the extension 5 seconds by default to calculate the refactoring edits. If you're refactoring a large project, it may take more than 5 seconds to calculate refactoring updates. When the timeout occurs, VS Code client will force the files to be renamed to the new location, which will cause the running refactoring job to throw an exception because the underlying files have changed.

I created an issue #111208 to discuss about onWillRenameFiles limitation.

[isidorn]

@testforstephen thanks for trying it out and for creating a new issue, let's continue the discussion there,

[isidorn]

Adding on-testplan since there is a related test plan item and this exact case will be verified by #111208