Skip to content

Commit

Permalink
update misleading target changing error message
Browse files Browse the repository at this point in the history
The "target file is changing" message is misleading because the
issue isn't that a target file  is changing, but that the source file is
undergoing multiple modifications within the same operation.

The check itself correctly flags the issue.
  • Loading branch information
ayoisaiah committed Jan 30, 2025
1 parent edb12e3 commit 9d81a4c
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 14 deletions.
2 changes: 1 addition & 1 deletion internal/status/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const (
OverwritingNewPath Status = "overwriting new path"
ForbiddenCharacters Status = "forbidden characters present"
FilenameLengthExceeded Status = "filename too long"
TargetFileChanging Status = "target file is changing"
SourceAlreadyRenamed Status = "source already renamed"
SourceNotFound Status = "source not found"
Ignored Status = "ignored"
)
2 changes: 1 addition & 1 deletion report/report_test/report_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ var filesWithConflicts = file.Changes{
{
Source: "1.txt",
Target: "2.txt",
Status: status.TargetFileChanging,
Status: status.SourceAlreadyRenamed,
},
{
Source: "nonexistent_file.txt",
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
[{"base_dir":"","target_dir":"","source":"original.txt","target":"","status":"empty filename","is_dir":false},{"base_dir":"","target_dir":"","source":"original.txt","target":"new_file.","status":"trailing periods present","is_dir":false},{"base_dir":"","target_dir":"","source":"file1.txt","target":"existing_file.txt","status":"target exists","is_dir":false},{"base_dir":"","target_dir":"","source":"original.txt","target":"new:file.txt","status":"forbidden characters present","is_dir":false},{"base_dir":"","target_dir":"","source":"file2.txt","target":"new_file.txt","status":"overwriting new path","is_dir":false},{"base_dir":"","target_dir":"","source":"original.txt","target":"this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length.txt","status":"filename too long","is_dir":false},{"base_dir":"","target_dir":"","source":"1.txt","target":"2.txt","status":"target file is changing","is_dir":false},{"base_dir":"","target_dir":"","source":"nonexistent_file.txt","target":"new_name.txt","status":"source not found","is_dir":false}]
[{"base_dir":"","target_dir":"","source":"original.txt","target":"","status":"empty filename","is_dir":false},{"base_dir":"","target_dir":"","source":"original.txt","target":"new_file.","status":"trailing periods present","is_dir":false},{"base_dir":"","target_dir":"","source":"file1.txt","target":"existing_file.txt","status":"target exists","is_dir":false},{"base_dir":"","target_dir":"","source":"original.txt","target":"new:file.txt","status":"forbidden characters present","is_dir":false},{"base_dir":"","target_dir":"","source":"file2.txt","target":"new_file.txt","status":"overwriting new path","is_dir":false},{"base_dir":"","target_dir":"","source":"original.txt","target":"this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length.txt","status":"filename too long","is_dir":false},{"base_dir":"","target_dir":"","source":"1.txt","target":"2.txt","status":"source already renamed","is_dir":false},{"base_dir":"","target_dir":"","source":"nonexistent_file.txt","target":"new_name.txt","status":"source not found","is_dir":false}]
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
| original.txt | new:file.txt | forbidden characters present |
| file2.txt | new_file.txt | overwriting new path |
| original.txt | this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length.txt | filename too long |
| 1.txt | 2.txt | target file is changing |
| 1.txt | 2.txt | source already renamed |
| nonexistent_file.txt | new_name.txt | source not found |
*——————————————————————*——————————————————————————————————————————————————————————————————————————*——————————————————————————————*
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
| original.txt | new:file.txt | forbidden characters present |
| file2.txt | new_file.txt | overwriting new path |
| original.txt | this_is_a_very_long_filename_that_exceeds_the_maximum_allowed_length.txt | filename too long |
| 1.txt | 2.txt | target file is changing |
| 1.txt | 2.txt | source already renamed |
| nonexistent_file.txt | new_name.txt | source not found |
*——————————————————————*——————————————————————————————————————————————————————————————————————————*——————————————————————————————*
12 changes: 6 additions & 6 deletions validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,10 @@ func checkPathExistsConflict(
return conflictDetected
}

// checkTargetFileChangingConflict ensures that renaming a file to a target that
// is changing later is detected to prevent data loss. It is automatically fixed
// by swapping the items around so that any renaming targets do not change later.
func checkTargetFileChangingConflict(
// checkSourceAlreadyRenamedConflict ensures that renaming a file multiple times
// is detected to prevent data loss. It is automatically fixed by swapping the
// items around so that any renaming targets do not change later.
func checkSourceAlreadyRenamedConflict(
ctx validationCtx,
) (conflictDetected bool) {
seenIndex, ok := ctx.seenPaths[ctx.change.SourcePath]
Expand All @@ -187,7 +187,7 @@ func checkTargetFileChangingConflict(
}

conflictDetected = true
ctx.change.Status = status.TargetFileChanging
ctx.change.Status = status.SourceAlreadyRenamed

if ctx.autoFix {
changes[seenIndex], changes[ctx.changeIndex] = changes[ctx.changeIndex], changes[seenIndex]
Expand Down Expand Up @@ -412,7 +412,7 @@ func checkAndHandleConflict(ctx validationCtx, loopIndex *int) (detected bool) {
checkPathExistsConflict,
checkOverwritingPathConflict,
checkSourceNotFoundConflict,
checkTargetFileChangingConflict, // INFO: Needs to be the last check
checkSourceAlreadyRenamedConflict, // INFO: Needs to be the last check
}

for i, check := range checks {
Expand Down
6 changes: 3 additions & 3 deletions validate/validate_test/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func TestValidate(t *testing.T) {
{
Source: "dsc-002.arw",
Target: "dsc-003.arw",
Status: status.TargetFileChanging,
Status: status.SourceAlreadyRenamed,
BaseDir: "testdata/images",
},
},
Expand Down Expand Up @@ -250,12 +250,12 @@ func TestValidate(t *testing.T) {
{
Source: "02.txt",
Target: "01.txt",
Status: status.TargetFileChanging,
Status: status.SourceAlreadyRenamed,
},
{
Source: "01.txt",
Target: "00.txt",
Status: status.TargetFileChanging,
Status: status.SourceAlreadyRenamed,
},
},
ConflictDetected: true,
Expand Down

0 comments on commit 9d81a4c

Please sign in to comment.