deprecate strutils.delete and add an overload with saner semantics consistent with sequtils.delete; follows #18487 (#18510)

3 files changed, 59 insertions, 9 deletions

diff --git a/changelog.md b/changelog.md
index 16f2192dc..6dddd8a01 100644
--- a/changelog.md
+++ b/changelog.md
@@ -346,7 +346,7 @@
 - Added `dom.setInterval`, `dom.clearInterval` overloads.
 
 - Deprecated `sequtils.delete` and added an overload taking a `Slice` that raises a defect
-  if the slice is out of bounds.
+  if the slice is out of bounds, likewise with `strutils.delete`.
 
 ## Language changes
 
diff --git a/lib/pure/strutils.nim b/lib/pure/strutils.nim
index 9e0837942..ac078df4e 100644
--- a/lib/pure/strutils.nim
+++ b/lib/pure/strutils.nim
@@ -1476,12 +1476,39 @@ func dedent*(s: string, count: Natural = indentation(s)): string {.rtl,
     doAssert x == "Hello\n  There\n"
   unindent(s, count, " ")
 
-func delete*(s: var string, first, last: int) {.rtl, extern: "nsuDelete".} =
-  ## Deletes in `s` (must be declared as `var`) the characters at positions
-  ## `first .. last` (both ends included).
-  ##
-  ## This modifies `s` itself, it does not return a copy.
-  runnableExamples:
+func delete*(s: var string, slice: Slice[int]) =
+  ## Deletes the items `s[slice]`, raising `IndexDefect` if the slice contains
+  ## elements out of range.
+  ##
+  ## This operation moves all elements after `s[slice]` in linear time, and
+  ## is the string analog to `sequtils.delete`.
+  runnableExamples:
+    var a = "abcde"
+    doAssertRaises(IndexDefect): a.delete(4..5)
+    assert a == "abcde"
+    a.delete(4..4)
+    assert a == "abcd"
+    a.delete(1..2)
+    assert a == "ad"
+    a.delete(1..<1) # empty slice
+    assert a == "ad"
+  when compileOption("boundChecks"):
+    if not (slice.a < s.len and slice.a >= 0 and slice.b < s.len):
+      raise newException(IndexDefect, $(slice: slice, len: s.len))
+  if slice.b >= slice.a:
+    var i = slice.a
+    var j = slice.b + 1
+    var newLen = s.len - j + i
+    # if j < s.len: moveMem(addr s[i], addr s[j], s.len - j) # pending benchmark
+    while i < newLen:
+      s[i] = s[j]
+      inc(i)
+      inc(j)
+    setLen(s, newLen)
+
+func delete*(s: var string, first, last: int) {.rtl, extern: "nsuDelete", deprecated: "use `delete(s, first..last)`".} =
+  ## Deletes in `s` the characters at positions `first .. last` (both ends included).
+  runnableExamples("--warning:deprecated:off"):
     var a = "abracadabra"
 
     a.delete(4, 5)
@@ -1502,7 +1529,6 @@ func delete*(s: var string, first, last: int) {.rtl, extern: "nsuDelete".} =
     inc(j)
   setLen(s, newLen)
 
-
 func startsWith*(s: string, prefix: char): bool {.inline.} =
   ## Returns true if `s` starts with character `prefix`.
   ##
diff --git a/tests/stdlib/tstrutils.nim b/tests/stdlib/tstrutils.nim
index 68ee5812b..234991bdb 100644
--- a/tests/stdlib/tstrutils.nim
+++ b/tests/stdlib/tstrutils.nim
@@ -198,7 +198,30 @@ template main() =
     s.removePrefix("")
     doAssert s == "\r\n\r\nhello"
 
-  block: # delete
+  block: # delete(slice)
+    var s = "0123456789ABCDEFGH"
+    delete(s, 4 .. 5)
+    doAssert s == "01236789ABCDEFGH"
+    delete(s, s.len-1 .. s.len-1)
+    doAssert s == "01236789ABCDEFG"
+    delete(s, 0..0)
+    doAssert s == "1236789ABCDEFG"
+    s = ""
+    doAssertRaises(IndexDefect): delete(s, 0..0)
+    doAssert s == ""
+    s = "abc"
+    doAssertRaises(IndexDefect): delete(s, -1 .. -2)
+    doAssertRaises(IndexDefect): delete(s, 2..3)
+    doAssertRaises(IndexDefect): delete(s, 3..2)
+    delete(s, 2..2)
+    doAssert s == "ab"
+    delete(s, 1..0)
+    doAssert s == "ab"
+    delete(s, 0..0)
+    doAssert s == "b"
+
+  block: # delete(first, last)
+    {.push warning[deprecated]:off.}
     var s = "0123456789ABCDEFGH"
     delete(s, 4, 5)
     doAssert s == "01236789ABCDEFGH"
@@ -206,6 +229,7 @@ template main() =
     doAssert s == "01236789ABCDEFG"
     delete(s, 0, 0)
     doAssert s == "1236789ABCDEFG"
+    {.pop.}
 
   block: # find
     doAssert "0123456789ABCDEFGH".find('A') == 10