From 66d672505a2780f92b9677d71fda6eeb88754cd8 Mon Sep 17 00:00:00 2001 From: Humza Shahid Date: Sun, 18 Jan 2026 00:27:03 +0000 Subject: [PATCH] fix calculation of number to decrement by in 'PersistentVector.delete', after additional test showed that calculation resulted in incorrect metadata. (Todo: just implement function to merge left and right subtrees.) --- fcore/persistent-vector.sml | 126 +++++++++++++++++-------------- test/persistent-vector-tests.sml | 33 ++++++++ todo.md | 3 +- 3 files changed, 103 insertions(+), 59 deletions(-) diff --git a/fcore/persistent-vector.sml b/fcore/persistent-vector.sml index cde2a9d..817f8ff 100644 --- a/fcore/persistent-vector.sml +++ b/fcore/persistent-vector.sml @@ -548,67 +548,77 @@ struct fun merge (left, right) = raise Fail "unimplemennted" fun delete (start, length, tree) = - let - val finish = start + length - val matchBeforeStart = prevMatch (start, tree, 1) - val matchAfterFinish = nextMatch (finish, tree, 1) - in - if matchBeforeStart = ~1 andalso matchAfterFinish = ~1 then - empty - else if matchAfterFinish = ~1 then - (* there is no match after 'finish', so just split left. - * No need to decrement or split right, - * because the right vector would be empty. *) - splitLeft (start, tree) - else if matchBeforeStart = ~1 then - (* We don't want to use left split - * because there are no elements - * before the deletion range's 'start'. - * So we make a right split and then decrement it. *) - let - val right = splitRight (finish, tree) - in - if isEmpty right then - empty + if isEmpty tree then + empty + else + let + val finish = start + length + val matchBeforeStart = prevMatch (start, tree, 1) + val matchBeforeStart = + if matchBeforeStart >= start then + ~1 else - let - val startIdx = getStartIdx right - val difference = matchAfterFinish - startIdx - in - if difference = 0 then - right - else if isEmpty right then - empty - else - decrementBy (difference, right) - end - end - else - let - val left = splitLeft (start, tree) - val right = splitRight (finish, tree) - in - if isEmpty right then - left - else - let - val leftSize = getFinishIdx left - val rightStartRelative = getStartIdx right + matchBeforeStart + val matchAfterFinish = nextMatch (finish, tree, 1) + in + if matchBeforeStart = ~1 andalso matchAfterFinish = ~1 then + empty + else if matchAfterFinish = ~1 then + (* there is no match after 'finish', so just split left. + * No need to decrement or split right, + * because the right vector would be empty. *) + splitLeft (start, tree) + else if matchBeforeStart = ~1 then + (* We don't want to use left split + * because there are no elements + * before the deletion range's 'start'. + * So we make a right split and then decrement it. *) + let + val right = splitRight (finish, tree) + in + if isEmpty right then + empty + else + let + val startIdx = getStartIdx right + val shouldBeStartIdx = matchAfterFinish - length + val difference = startIdx - shouldBeStartIdx + in + if difference = 0 then + right + else if isEmpty right then + empty + else + decrementBy (difference, right) + end + end + else + let + val left = splitLeft (start, tree) + val right = splitRight (finish, tree) + in + if isEmpty right then + left + else + let + val leftSize = getFinishIdx left + val rightStartRelative = getStartIdx right + val rightStartAbsolute = leftSize + rightStartRelative - val rightStartAbsolute = leftSize + rightStartRelative - val difference = matchAfterFinish - rightStartAbsolute - in - if difference = 0 then - merge (left, right) - else - let - val right = decrementBy (difference, right) - in + val shouldBeStartIdx = matchAfterFinish - length + val difference = rightStartAbsolute - shouldBeStartIdx + in + if difference = 0 then merge (left, right) - end - end - end - end + else + let + val right = decrementBy (difference, right) + in + merge (left, right) + end + end + end + end (* functions only for testing *) fun fromListLoop (lst, acc) = diff --git a/test/persistent-vector-tests.sml b/test/persistent-vector-tests.sml index 17114d5..341b94b 100644 --- a/test/persistent-vector-tests.sml +++ b/test/persistent-vector-tests.sml @@ -342,6 +342,39 @@ struct in Expect.isTrue (outputList = expectedOutput) end) + , test + "decrements subsequent elements correctly \ + \when deletion range is before first element to middle element" + (fn _ => + let + (* arrange *) + val inputList = + [ {start = 1, finish = 1} + , {start = 2, finish = 2} + , {start = 3, finish = 3} + , {start = 4, finish = 4} + , {start = 50, finish = 50} + , {start = 60, finish = 60} + , {start = 70, finish = 70} + , {start = 80, finish = 80} + ] + val pv = PersistentVector.fromList inputList + + (* act *) + val pv = PersistentVector.delete (0, 3, pv) + + (* assert *) + val outputList = PersistentVector.toList pv + val expectedOutput = + [ {start = 1, finish = 1} + , {start = 47, finish = 47} + , {start = 57, finish = 57} + , {start = 67, finish = 67} + , {start = 77, finish = 77} + ] + in + Expect.isTrue (outputList = expectedOutput) + end) ] val tests = [appendTests, toListTests, splitLeftTests, deleteTests] diff --git a/todo.md b/todo.md index 5b5b449..e497af4 100644 --- a/todo.md +++ b/todo.md @@ -1,3 +1,4 @@ # To-do list -- Implement `PersistentVector.delete` and test it +- Implement function to merge left and right subtrees for `PersistentVector.delete` function +- Test `PersistentVector.delete` function - Implement 'yj' motion and add tests for it