Skip to content

Preserve uncommitted file changes during revert and cherry-pick - #9110

Merged
hansva merged 2 commits into
apache:mainfrom
dajiaohuang:fix/git-file-actions-preserve-local-changes-8935
Oct 28, 2026
Merged

hansva merged 2 commits into
apache:mainfrom
dajiaohuang:fix/git-file-actions-preserve-local-changes-8935

Conversation

@dajiaohuang

Copy link
Copy Markdown
Contributor

Summary

The Git perspective's file-level Revert and Cherry-pick actions both use UIGit.restorePathFromCommit, which could replace the selected file while staged or unstaged local edits were present. Check the selected path's JGit status before restoring or removing it and refuse with a commit-or-stash message when that path is dirty. Unrelated dirty files do not block the operation.

Fixes #8935.

Verification

  • mvnw.cmd -B -pl plugins/misc/git spotless:apply
  • mvnw.cmd -B -Pskip-uitest -pl plugins/misc/git -am -Dtest=UIGitTest -Dsurefire.failIfNoSpecifiedTests=false test (Java 21): 63 tests passed, 0 failures, 0 errors.

@bamaer bamaer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, @dajiaohuang. Checking the path's status inside restorePathFromCommit covers both file actions and the git rm branch, and isClean() is the right check: it also catches an untracked file at the path, which checkout would overwrite.

Refuse before the confirmation, not after

Right now the user confirms "Revert 'x' to the version before commit …?", clicks Yes, and then gets an ErrorDialog with a stack trace. This is a precondition, not an error. Please:

  • Add a public helper to UIGit, e.g. hasLocalChanges(String path), using the same status().addPath(..).call().isClean() check.
  • In GitPerspective.revertFile() and cherryPickFile(), call it before confirmFileAction and show a warning MessageBox. Load its text from messages_en_US.properties, for example ''{0}'' has uncommitted changes. Commit or stash them first.
  • Keep the check in restorePathFromCommit as a backstop.

Tests

  • Another dirty file does not block the restore. The description says so, but no test covers it.
  • Reverting a commit that added the file while that file has local edits. This is the git rm branch.
  • An untracked file at the path when reverting a commit that deleted it.
  • testRestorePathFromCommitPreservesStagedChanges: assert the message as the unstaged test does, so that an unrelated exception does not pass the test.

Optional, pre-existing

revertCommit() and cherryPickCommit() build the "repository is not clean" MessageBox but never call open(). On a dirty repository those actions return without saying anything. It's a one-line fix in each method if you're in this code anyway; otherwise it can go in a separate issue.

@dajiaohuang

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I pushed commit 1b4d9135346f0cbc51d7d8b7ee9e737c0e23e9c3 to the existing PR branch to check path-scoped local changes before the confirmation dialog for file revert/cherry-pick, with a localized warning; the restore helper still checks as a backstop. The tests now cover another dirty path, local edits on a file the selected commit added, an untracked file where a revert would restore a deleted path, and the staged-change exception message. I also made the existing dirty-repository warnings for commit-level revert/cherry-pick visible.

Validation: the targeted Maven build completed successfully; UIGitTest ran 66 tests with 0 failures/errors/skips. git diff --check passed. Compilation reported existing warnings; CI was not queried.

@hansva
hansva merged commit ad6314a into apache:main Oct 28, 2026
3 checks passed
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

Successfully merging this pull request may close these issues.

[Bug]: Revert file and Cherry-pick file overwrite uncommitted changes without warning

3 participants