Skip to content

fix: Fix undo/redo functionality and tests in Maven/Tycho environment #45

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 7, 2025
Merged

fix: Fix undo/redo functionality and tests in Maven/Tycho environment #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
657753c
fix: Fix undo/redo functionality and tests in Maven/Tycho environment
Artur- Sep 5, 2025
fe30961
Merge branch 'main' into undoredo-tests
manolo Sep 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 81 additions & 30 deletions vaadin-eclipse-plugin-main/src/com/vaadin/plugin/CopilotUndoManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
public class CopilotUndoManager {

private static CopilotUndoManager instance;
private static final IUndoContext WORKSPACE_CONTEXT = new WorkspaceUndoContext();
private IOperationHistory operationHistory;
private Map<String, List<IUndoableOperation>> fileOperations;

Expand All @@ -54,11 +55,19 @@ public void recordOperation(IFile file, String oldContent, String newContent, St
* Record a file modification operation for undo/redo with binary flag.
*/
public void recordOperation(IFile file, String oldContent, String newContent, String label, boolean isBase64) {
// Create operation but don't execute it since content was already changed
CopilotFileEditOperation operation = new CopilotFileEditOperation(file, oldContent, newContent, label,
isBase64);

try {
operationHistory.execute(operation, null, null);
// The file content has already been changed externally
// We add the operation to the history in executed state
IUndoContext context = getOrCreateWorkspaceContext();

if (context != null) {
operation.addContext(context);
}
operationHistory.add(operation);

// Track operation for this file
String filePath = file.getFullPath().toString();
Expand All @@ -70,6 +79,22 @@ public void recordOperation(IFile file, String oldContent, String newContent, St
}
}

/**
* Get or create a workspace undo context. This handles cases where the workspace adapter might not be available.
*/
private IUndoContext getOrCreateWorkspaceContext() {
// First try to get the workspace context
IUndoContext context = ResourcesPlugin.getWorkspace().getAdapter(IUndoContext.class);

if (context == null) {
// If no workspace context, use our singleton custom context
// This ensures tests can run even without full workspace setup
context = WORKSPACE_CONTEXT;
}

return context;
}

/**
* Perform undo for specified files.
*/
Expand All @@ -81,18 +106,11 @@ public boolean performUndo(List<String> filePaths) {
IFile file = findFile(filePath);
if (file != null) {
// Get the workspace context which our operations use
IUndoContext context = ResourcesPlugin.getWorkspace().getAdapter(IUndoContext.class);
System.out.println("Undo attempt for: " + filePath + ", context: " + context);
if (context != null) {
boolean canUndo = operationHistory.canUndo(context);
System.out.println("Can undo: " + canUndo);
if (canUndo) {
IStatus status = operationHistory.undo(context, null, null);
System.out.println("Undo status: " + status.isOK() + ", message: " + status.getMessage());
if (status.isOK()) {
performed = true;
System.out.println("Undo performed for: " + filePath);
}
IUndoContext context = getOrCreateWorkspaceContext();
if (context != null && operationHistory.canUndo(context)) {
IStatus status = operationHistory.undo(context, null, null);
if (status.isOK()) {
performed = true;
}
}
}
Expand All @@ -116,18 +134,11 @@ public boolean performRedo(List<String> filePaths) {
IFile file = findFile(filePath);
if (file != null) {
// Get the workspace context which our operations use
IUndoContext context = ResourcesPlugin.getWorkspace().getAdapter(IUndoContext.class);
System.out.println("Redo attempt for: " + filePath + ", context: " + context);
if (context != null) {
boolean canRedo = operationHistory.canRedo(context);
System.out.println("Can redo: " + canRedo);
if (canRedo) {
IStatus status = operationHistory.redo(context, null, null);
System.out.println("Redo status: " + status.isOK() + ", message: " + status.getMessage());
if (status.isOK()) {
performed = true;
System.out.println("Redo performed for: " + filePath);
}
IUndoContext context = getOrCreateWorkspaceContext();
if (context != null && operationHistory.canRedo(context)) {
IStatus status = operationHistory.redo(context, null, null);
if (status.isOK()) {
performed = true;
}
}
}
Expand Down Expand Up @@ -184,6 +195,26 @@ private IUndoContext getUndoContext(IFile file) {
return ResourcesPlugin.getWorkspace().getAdapter(IUndoContext.class);
}

/**
* Custom workspace undo context for when the default one isn't available. This is primarily for test environments
* where the workspace might not be fully initialized.
*/
private static class WorkspaceUndoContext implements IUndoContext {
private static final String LABEL = "Copilot Workspace Context";

@Override
public String getLabel() {
return LABEL;
}

@Override
public boolean matches(IUndoContext context) {
// Match with itself or other workspace contexts
return context == this || context instanceof WorkspaceUndoContext
|| (context != null && LABEL.equals(context.getLabel()));
}
}

/**
* Custom undoable operation for Copilot file edits.
*/
Expand All @@ -194,6 +225,7 @@ private static class CopilotFileEditOperation implements IUndoableOperation {
private final String newContent;
private final String label;
private final boolean isBase64;
private IUndoContext[] contexts;

public CopilotFileEditOperation(IFile file, String oldContent, String newContent, String label) {
this(file, oldContent, newContent, label, false);
Expand All @@ -206,10 +238,14 @@ public CopilotFileEditOperation(IFile file, String oldContent, String newContent
this.newContent = newContent;
this.label = label != null ? label : "Copilot Edit";
this.isBase64 = isBase64;
// Initialize with empty contexts, will be added during recordOperation
this.contexts = new IUndoContext[0];
}

@Override
public IStatus execute(IProgressMonitor monitor, IAdaptable info) {
// Should not be called since content is already applied
// But if it is called, apply the new content
return setFileContent(newContent);
}

Expand Down Expand Up @@ -270,13 +306,11 @@ public String getLabel() {

@Override
public IUndoContext[] getContexts() {
IUndoContext context = ResourcesPlugin.getWorkspace().getAdapter(IUndoContext.class);
return context != null ? new IUndoContext[] { context } : new IUndoContext[0];
return contexts;
}

@Override
public boolean hasContext(IUndoContext context) {
IUndoContext[] contexts = getContexts();
for (IUndoContext c : contexts) {
if (c.matches(context)) {
return true;
Expand All @@ -287,12 +321,29 @@ public boolean hasContext(IUndoContext context) {

@Override
public void addContext(IUndoContext context) {
// Not needed for our use case
// Add context if not already present
for (IUndoContext c : contexts) {
if (c.matches(context)) {
return; // Already has this context
}
}
// Create new array with added context
IUndoContext[] newContexts = new IUndoContext[contexts.length + 1];
System.arraycopy(contexts, 0, newContexts, 0, contexts.length);
newContexts[contexts.length] = context;
contexts = newContexts;
}

@Override
public void removeContext(IUndoContext context) {
// Not needed for our use case
// Not needed for our use case but implemented for completeness
List<IUndoContext> remaining = new ArrayList<>();
for (IUndoContext c : contexts) {
if (!c.matches(context)) {
remaining.add(c);
}
}
contexts = remaining.toArray(new IUndoContext[0]);
}

@Override
Expand Down
54 changes: 28 additions & 26 deletions vaadin-eclipse-plugin.tests/src/com/vaadin/plugin/test/CopilotUndoManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ public void testRecordAndUndoOperation() throws Exception {

file.create(new java.io.ByteArrayInputStream(originalContent.getBytes("UTF-8")), true, null);

// Record an operation
undoManager.recordOperation(file, originalContent, newContent, "Test modification");

// Apply the new content
// Apply the new content first (simulating what REST service does)
file.setContents(new java.io.ByteArrayInputStream(newContent.getBytes("UTF-8")), true, true, null);

// Then record the operation for undo
undoManager.recordOperation(file, originalContent, newContent, "Test modification");

// Verify new content
String currentContent = readFileContent(file);
assertEquals("File should have new content", newContent, currentContent);
Expand All @@ -70,9 +70,9 @@ public void testRecordAndRedoOperation() throws Exception {

file.create(new java.io.ByteArrayInputStream(originalContent.getBytes("UTF-8")), true, null);

// Record and apply operation
undoManager.recordOperation(file, originalContent, modifiedContent, "Modify");
// Apply and then record operation
file.setContents(new java.io.ByteArrayInputStream(modifiedContent.getBytes("UTF-8")), true, true, null);
undoManager.recordOperation(file, originalContent, modifiedContent, "Modify");

// Undo
List<String> filePaths = Arrays.asList(file.getLocation().toString());
Expand All @@ -99,13 +99,13 @@ public void testMultipleOperations() throws Exception {

file.create(new java.io.ByteArrayInputStream(content1.getBytes("UTF-8")), true, null);

// Record first operation
undoManager.recordOperation(file, content1, content2, "First edit");
// Apply first change then record
file.setContents(new java.io.ByteArrayInputStream(content2.getBytes("UTF-8")), true, true, null);
undoManager.recordOperation(file, content1, content2, "First edit");

// Record second operation
undoManager.recordOperation(file, content2, content3, "Second edit");
// Apply second change then record
file.setContents(new java.io.ByteArrayInputStream(content3.getBytes("UTF-8")), true, true, null);
undoManager.recordOperation(file, content2, content3, "Second edit");

assertEquals("Should have version 3", content3, readFileContent(file));

Expand Down Expand Up @@ -158,18 +158,18 @@ public void testFileCreationUndo() throws Exception {
IFile file = testProject.getFile("create-undo.txt");
String newContent = "Created file";

// Simulate file creation by recording with empty old content
undoManager.recordOperation(file, "", newContent, "Create file");

// Create the actual file
// Create the actual file first
file.create(new java.io.ByteArrayInputStream(newContent.getBytes("UTF-8")), true, null);

// Then record the creation for undo
undoManager.recordOperation(file, "", newContent, "Create file");
assertTrue("File should exist", file.exists());

// Undo should set content to empty (can't delete via content operation)
List<String> filePaths = Arrays.asList(file.getLocation().toString());
boolean undone = undoManager.performUndo(filePaths);

assertTrue("Undo should be performed", undone);
assertTrue("CUndo should be performed", undone);
assertEquals("File content should be empty after undo", "", readFileContent(file));
}

Expand All @@ -182,20 +182,23 @@ public void testFileDeletionUndo() throws Exception {
// Create file
file.create(new java.io.ByteArrayInputStream(originalContent.getBytes("UTF-8")), true, null);

// Note: We still need the file to exist to perform undo operations
// Set content to empty instead of deleting
file.setContents(new java.io.ByteArrayInputStream("".getBytes("UTF-8")), true, true, null);

// Record deletion (new content is empty)
undoManager.recordOperation(file, originalContent, "", "Delete file");

// Delete the file
file.delete(true, null);
assertFalse("File should not exist after deletion", file.exists());
assertTrue("File should still exist but with empty content", file.exists());
assertEquals("File should have empty content", "", readFileContent(file));

// Try to undo - this will fail because file doesn't exist
// Try to undo - should restore original content
List<String> filePaths = Arrays.asList(file.getLocation().toString());
boolean undone = undoManager.performUndo(filePaths);

// Note: This will be false because the file doesn't exist
// The undo manager needs the file to exist to restore content
assertFalse("Undo cannot be performed on deleted file", undone);
// Should work now since file still exists
assertTrue("Undo should be performed", undone);
assertEquals("File content should be restored", originalContent, readFileContent(file));
}

@Test
Expand All @@ -212,13 +215,12 @@ public void testMultipleFilesUndo() throws Exception {
file1.create(new java.io.ByteArrayInputStream(original1.getBytes("UTF-8")), true, null);
file2.create(new java.io.ByteArrayInputStream(original2.getBytes("UTF-8")), true, null);

// Record operations for both files
// Apply changes first, then record operations
file1.setContents(new java.io.ByteArrayInputStream(modified1.getBytes("UTF-8")), true, true, null);
undoManager.recordOperation(file1, original1, modified1, "Modify file1");
undoManager.recordOperation(file2, original2, modified2, "Modify file2");

// Apply changes
file1.setContents(new java.io.ByteArrayInputStream(modified1.getBytes("UTF-8")), true, true, null);
file2.setContents(new java.io.ByteArrayInputStream(modified2.getBytes("UTF-8")), true, true, null);
undoManager.recordOperation(file2, original2, modified2, "Modify file2");

// Undo both
List<String> filePaths = Arrays.asList(file1.getLocation().toString(), file2.getLocation().toString());
Expand Down
Loading