From 10355cb48a0e98a448594ed29faa0b2d73391ee7 Mon Sep 17 00:00:00 2001
From: ee7 <45465154+ee7@users.noreply.github.com>
Date: Wed, 5 Oct 2022 19:57:17 +0200
Subject: strutils, rstgen: avoid deprecated `strutils.delete` (#20488)

The strutils `delete` func with signature

    func delete*(s: var string, first, last: int)

was deprecated in adba5eb45e0a, in favor of one with signature

    func delete*(s: var string, slice: Slice[int])

However, a few procedures still used the deprecated form. This commit
updates them, resolving these deprecation warnings:

    rstgen.nim(766, 12) Warning: use `delete(s, first..last)`; delete is deprecated [Deprecated]
    strutils.nim(1651, 19) Warning: use `delete(s, first..last)`; delete is deprecated [Deprecated]
    strutils.nim(1679, 7) Warning: use `delete(s, first..last)`; delete is deprecated [Deprecated]
    strutils.nim(2472, 7) Warning: use `delete(s, first..last)`; delete is deprecated [Deprecated]

Before this commit:

- `trimZeros` called `s.delete(i+1, i)` for an input that lacks a
  trailing zero (like "1.23").

- `removePrefix*(s: var string, prefix: string)` called
  `s.delete(0, -1)` when the prefix was the empty string.

which did not modify `s`, nor raise an error. But the newer slice
`delete` raises an `IndexDefect` when the start of the slice is greater
than the end, so we avoid calling the new `delete` for such a case.

Recall that exceptions inheriting from `system.Defect` are not tracked
with the `.raises: []` exception tracking mechanism [1], so this commit
does not break existing code like:

    proc foo {.raises: [].} =
      var s = "abc1.20"
      s.removePrefix("abc")
      s.trimZeros()
      doAssert s == "1.2"

The `strutils.delete` deprecation was motivated by a problem with
`system.delete` [2][3]:

    `system.delete` had surprising behavior when the index passed to it
    was out of bounds (it would delete the last entry then). Compile
    with `-d:nimStrictDelete` so that an index error is produced
    instead.

    Be aware however that your code might depend on this quirky behavior
    so a review process is required on your part before you can use
    `-d:nimStrictDelete`. To make this review easier, use the
    `-d:nimAuditDelete` switch, which pretends that `system.delete` is
    deprecated so that it is easier to see where it was used in your
    code.

    `-d:nimStrictDelete` will become the default in upcoming versions.

A similar deprecation happened with `sequtils.delete` [4], but that
deprecated form is already not used in this repo.

[1] https://github.com/nim-lang/Nim/blob/2dec69fe5aa6/doc/manual.md#exception-tracking
[2] https://github.com/nim-lang/Nim/blob/2dec69fe5aa6/changelogs/changelog_1_6_0.md#system
[3] https://github.com/nim-lang/Nim/commit/92cb76571432
[4] https://github.com/nim-lang/Nim/commit/1d6863a7899f
---
 tests/stdlib/tstrutils.nim | 3 +++
 1 file changed, 3 insertions(+)

(limited to 'tests/stdlib/tstrutils.nim')

diff --git a/tests/stdlib/tstrutils.nim b/tests/stdlib/tstrutils.nim
index 32929ef17..4e88197ff 100644
--- a/tests/stdlib/tstrutils.nim
+++ b/tests/stdlib/tstrutils.nim
@@ -433,6 +433,9 @@ template main() =
     x = "1e0"
     x.trimZeros()
     doAssert x == "1e0"
+    x = "1.23"
+    x.trimZeros()
+    doAssert x == "1.23"
 
   block: # countLines
     proc assertCountLines(s: string) = doAssert s.countLines == s.splitLines.len
-- 
cgit 1.4.1-2-gfad0