fix(ADFA-2584): Allow file rename for case-only changes#1359
Open
dara-abijo-adfa wants to merge 1 commit into
Open
fix(ADFA-2584): Allow file rename for case-only changes#1359dara-abijo-adfa wants to merge 1 commit into
dara-abijo-adfa wants to merge 1 commit into
Conversation
dara-abijo-adfa
requested review from
Daniel-ADFA,
itsaky-adfa,
jatezzz and
jomen-adfa
jomen-adfa
approved these changes
Jun 2, 2026
jatezzz
approved these changes
Jun 2, 2026
Contributor
📝 WalkthroughRelease NotesNew Feature
Implementation Details
|
| Layer / File(s) | Summary |
|---|---|
Rename logic with case-insensitive support app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt |
The rename operation creates a destination File and branches: if the desired name matches the current name case-insensitively, it renames through a temporary *.tmp intermediate file; otherwise it directly renames via FileUtils.rename. Name length validation (1..40 characters) is moved to the final success condition. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
- appdevforall/CodeOnTheGo#1264: Both PRs modify
FileManagerViewModel.renameFile, with this PR refining the rename and validation logic while the related PR changed method signature and event handling.
Suggested reviewers
- itsaky-adfa
- Daniel-ADFA
- jomen-adfa
Poem
🐰 A file seeks to change its case with grace,
Through temp files dancing in rename's embrace,
No length too long for our hop and sprint,
Just validate when the deed's complete—that's the hint!
File Manager hops on, bug fixed with a bound! 🎉
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically summarizes the main change: allowing file rename operations when only the case differs. |
| Description check | ✅ Passed | The description is directly related to the changeset, providing a clear example of the use case being addressed. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
ADFA-2584-allow-case-only-file-rename
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt (2)
33-38: 💤 Low valueOptional: Optimize for exact name match case.
When
file.nameequalsnewNameexactly (not just case-insensitively), this is a no-op rename. The current implementation still performs the two-step rename through a temporary file, which is unnecessary. Consider returning early with success when the names match exactly.⚡ Proposed optimization
+if (file.name == newName) { + return@withContext true +} if (file.name.equals(newName, ignoreCase = true)) { val tempFile = File(file.parentFile, "$newName.tmp") file.renameTo(tempFile) && tempFile.renameTo(destFile) } else { FileUtils.rename(file, newName) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt` around lines 33 - 38, Detect the exact-match no-op before doing the temporary rename: if file.name == newName (case-sensitive) simply return success instead of performing the two-step temp rename; update the logic in the FileManagerViewModel rename flow (the block that currently checks file.name.equals(newName, ignoreCase = true) and uses tempFile/destFile) to first check for exact equality and short-circuit, otherwise fall back to the existing temp-file sequence or FileUtils.rename call.
43-43: 💤 Low valueAlign
FileRenameEventconstruction with rename destination (nofile.parentNPE risk)
File(file.parent, newName)won’t fail due tofile.parentbeing nullable—java.io.File(String, String)treats a nullparentasnew File(child). Still, for consistency with the actual rename path (destFile = File(file.parentFile, newName)), build the event usingdestFile(orFile(file.parentFile, newName)) instead ofFile(file.parent, newName)atFileManagerViewModel.kt:43.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt` at line 43, The FileRenameEvent is constructed with File(file.parent, newName) which is inconsistent with the actual rename target (destFile) and risks mismatch; update the creation of renameEvent in FileManagerViewModel (the variable renameEvent) to use the same destFile used for renaming (or construct with File(file.parentFile, newName)) so the event accurately reflects the destination path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt`:
- Around line 34-35: The rename sequence uses a fixed temp filename
("$newName.tmp") which can collide; update the logic in FileManagerViewModel
around variables tempFile, file.renameTo(tempFile) and destFile to first ensure
a unique temporary path (e.g., use File.createTempFile or loop to append an
increasing suffix until a non-existent temp is found), fail fast if you cannot
reserve a unique temp, perform file.renameTo(tempFile) then
tempFile.renameTo(destFile), and ensure you clean up the tempFile on any failure
to avoid leaving or overwriting files.
- Around line 34-35: The two-step rename in FileManagerViewModel (where it
creates tempFile and calls file.renameTo(tempFile) &&
tempFile.renameTo(destFile)) lacks rollback: if the first rename succeeds but
the second fails the temp file remains. Change the logic in the renaming method
in FileManagerViewModel to perform the second rename in an if block so you can
detect failure; on failure attempt to move tempFile back to the original file
name (rollback), catch/log any exceptions (use whatever logger is available in
this class), and return false; ensure the method returns true only if both
renames succeed and that all filesystem operations are guarded and error-handled
to avoid leaving a .tmp file behind.
- Line 41: The rename validation is happening after the file is already renamed,
causing filesystem changes when the new name is invalid; in FileManagerViewModel
(the method handling rename—e.g., renameFile or the function where "renamed" and
"newName" are used) move the check newName.length in 1..40 to before any
file.renameTo / rename operation, return early or set an error/result when the
name is invalid, and only perform the actual rename when the name passes
validation (then update the existing condition that currently reads "if
(newName.length in 1..40 && renamed)" so it only evaluates renamed after a
successful pre-check).
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt`:
- Around line 33-38: Detect the exact-match no-op before doing the temporary
rename: if file.name == newName (case-sensitive) simply return success instead
of performing the two-step temp rename; update the logic in the
FileManagerViewModel rename flow (the block that currently checks
file.name.equals(newName, ignoreCase = true) and uses tempFile/destFile) to
first check for exact equality and short-circuit, otherwise fall back to the
existing temp-file sequence or FileUtils.rename call.
- Line 43: The FileRenameEvent is constructed with File(file.parent, newName)
which is inconsistent with the actual rename target (destFile) and risks
mismatch; update the creation of renameEvent in FileManagerViewModel (the
variable renameEvent) to use the same destFile used for renaming (or construct
with File(file.parentFile, newName)) so the event accurately reflects the
destination path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d1e6559-5286-456a-86f8-cb92064e4fb2
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt
Comment on lines
+34
to
+35
| val tempFile = File(file.parentFile, "$newName.tmp") | ||
| file.renameTo(tempFile) && tempFile.renameTo(destFile) |
Contributor
There was a problem hiding this comment.
Critical: Check for temporary file collision before rename.
The temporary file "$newName.tmp" could already exist in the directory. If it does, renameTo will either fail (leaving the original file unchanged) or potentially overwrite existing data on some file systems, causing data loss.
🛡️ Proposed fix to check for temp file existence
if (file.name.equals(newName, ignoreCase = true)) {
val tempFile = File(file.parentFile, "$newName.tmp")
+ if (tempFile.exists()) {
+ return@withContext false
+ }
file.renameTo(tempFile) && tempFile.renameTo(destFile)
} else {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val tempFile = File(file.parentFile, "$newName.tmp") | |
| file.renameTo(tempFile) && tempFile.renameTo(destFile) | |
| val tempFile = File(file.parentFile, "$newName.tmp") | |
| if (tempFile.exists()) { | |
| return@withContext false | |
| } | |
| file.renameTo(tempFile) && tempFile.renameTo(destFile) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt`
around lines 34 - 35, The rename sequence uses a fixed temp filename
("$newName.tmp") which can collide; update the logic in FileManagerViewModel
around variables tempFile, file.renameTo(tempFile) and destFile to first ensure
a unique temporary path (e.g., use File.createTempFile or loop to append an
increasing suffix until a non-existent temp is found), fail fast if you cannot
reserve a unique temp, perform file.renameTo(tempFile) then
tempFile.renameTo(destFile), and ensure you clean up the tempFile on any failure
to avoid leaving or overwriting files.
Major: Add rollback mechanism for failed two-step rename.
If file.renameTo(tempFile) succeeds but tempFile.renameTo(destFile) fails, the file will be left with the .tmp extension. There is no cleanup or rollback, leaving the file system in an inconsistent state.
🔄 Proposed fix to add rollback logic
if (file.name.equals(newName, ignoreCase = true)) {
val tempFile = File(file.parentFile, "$newName.tmp")
- file.renameTo(tempFile) && tempFile.renameTo(destFile)
+ val firstRename = file.renameTo(tempFile)
+ if (firstRename) {
+ val secondRename = tempFile.renameTo(destFile)
+ if (!secondRename) {
+ // Rollback: restore original name
+ tempFile.renameTo(file)
+ }
+ secondRename
+ } else {
+ false
+ }
} else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt`
around lines 34 - 35, The two-step rename in FileManagerViewModel (where it
creates tempFile and calls file.renameTo(tempFile) &&
tempFile.renameTo(destFile)) lacks rollback: if the first rename succeeds but
the second fails the temp file remains. Change the logic in the renaming method
in FileManagerViewModel to perform the second rename in an if block so you can
detect failure; on failure attempt to move tempFile back to the original file
name (rollback), catch/log any exceptions (use whatever logger is available in
this class), and return false; ensure the method returns true only if both
renames succeed and that all filesystem operations are guarded and error-handled
to avoid leaving a .tmp file behind.
| } | ||
|
|
||
| if (renamed) { | ||
| if (newName.length in 1..40 && renamed) { |
Contributor
There was a problem hiding this comment.
Critical: Validate name before performing rename operation.
The validation of newName.length in 1..40 now occurs AFTER the rename operation completes. This means if the name length is invalid (empty, or >40 characters), the file will still be renamed, but the operation will be reported as failed. This creates an inconsistent state where the file system changes but the operation is reported as an error.
Validation must occur before attempting the rename to prevent this inconsistency.
🛡️ Proposed fix to validate before rename
fun renameFile(file: File, newName: String, context: Context? = null, onResult: ((Boolean) -> Unit)? = null) {
viewModelScope.launch {
+ if (newName.length !in 1..40) {
+ _operationResult.emit(FileOpResult.Error(R.string.rename_failed))
+ onResult?.invoke(false)
+ return@launch
+ }
val destFile = File(file.parentFile, newName)
val renamed = withContext(Dispatchers.IO) {
if (file.name.equals(newName, ignoreCase = true)) {
val tempFile = File(file.parentFile, "$newName.tmp")
file.renameTo(tempFile) && tempFile.renameTo(destFile)
} else {
FileUtils.rename(file, newName)
}
}
- if (newName.length in 1..40 && renamed) {
+ if (renamed) {
// Notify system of the rename🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt` at
line 41, The rename validation is happening after the file is already renamed,
causing filesystem changes when the new name is invalid; in FileManagerViewModel
(the method handling rename—e.g., renameFile or the function where "renamed" and
"newName" are used) move the check newName.length in 1..40 to before any
file.renameTo / rename operation, return early or set an error/result when the
name is invalid, and only perform the actual rename when the name passes
validation (then update the existing condition that currently reads "if
(newName.length in 1..40 && renamed)" so it only evaluates renamed after a
successful pre-check).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow file renaming for case-only changes, e.g "numberFragment" to "NumberFragment"