Skip to content

Commit

Permalink
AK+Tests: Correct off-by-one error when right-trimming text
Browse files Browse the repository at this point in the history
If the entire string you want to right-trim consists of characters you
want to remove, we previously would incorrectly leave the first
character there.

For example: `trim("aaaaa", "a")` would return "a" instead of "".

We can't use `i >= 0` in the loop since that would fail to detect
underflow, so instead we keep `i` in the range `size .. 1` and then
subtract 1 from it when reading the character.

Added some trim() tests while I was at it. (And to confirm that this was
the issue.)
  • Loading branch information
AtkinsSJ authored and awesomekling committed Oct 11, 2022
1 parent 8202bee commit a0d4402
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
4 changes: 2 additions & 2 deletions AK/StringUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,10 @@ StringView trim(StringView str, StringView characters, TrimMode mode)
}

if (mode == TrimMode::Right || mode == TrimMode::Both) {
for (size_t i = str.length() - 1; i > 0; --i) {
for (size_t i = str.length(); i > 0; --i) {
if (substring_length == 0)
return ""sv;
if (!characters.contains(str[i]))
if (!characters.contains(str[i - 1]))
break;
--substring_length;
}
Expand Down
11 changes: 11 additions & 0 deletions Tests/AK/TestStringUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,17 @@ TEST_CASE(is_whitespace)
EXPECT(!AK::StringUtils::is_whitespace("a\t"sv));
}

TEST_CASE(trim)
{
EXPECT_EQ(AK::StringUtils::trim("aaa.a."sv, "."sv, TrimMode::Right), "aaa.a"sv);
EXPECT_EQ(AK::StringUtils::trim("...aaa"sv, "."sv, TrimMode::Left), "aaa"sv);
EXPECT_EQ(AK::StringUtils::trim("...aaa.a..."sv, "."sv, TrimMode::Both), "aaa.a"sv);
EXPECT_EQ(AK::StringUtils::trim("."sv, "."sv, TrimMode::Right), ""sv);
EXPECT_EQ(AK::StringUtils::trim("."sv, "."sv, TrimMode::Left), ""sv);
EXPECT_EQ(AK::StringUtils::trim("."sv, "."sv, TrimMode::Both), ""sv);
EXPECT_EQ(AK::StringUtils::trim("..."sv, "."sv, TrimMode::Both), ""sv);
}

TEST_CASE(find)
{
String test_string = "1234567";
Expand Down

0 comments on commit a0d4402

Please sign in to comment.