AK+UI: Stop using StringBuilder UTF-16 mode
Move the remaining call sites that constructed StringBuilder in UTF-16 mode over to Utf16StringBuilder or explicit UTF-8 to UTF-16 conversion. Teach FormatBuilder to target Utf16StringBuilder directly so formatted Utf16String construction no longer needs StringBuilder's UTF-16 mode. Update the Utf16String tests to cover Utf16StringBuilder adoption and clear behavior directly.
This commit is contained in:
Andreas Kling
Andreas Kling
parent
26b0801a78
commit
a8193e7572
9 changed files with 114 additions and 70 deletions
|
|
@ -217,16 +217,40 @@ bool FormatParser::consume_replacement_field(size_t& index)
|
|||
return true;
|
||||
}
|
||||
|
||||
ErrorOr<void> FormatBuilder::append(char ch)
|
||||
{
|
||||
if (m_string_builder)
|
||||
return m_string_builder->try_append(ch);
|
||||
m_utf16_builder->append_ascii(ch);
|
||||
return {};
|
||||
}
|
||||
|
||||
ErrorOr<void> FormatBuilder::append(StringView string)
|
||||
{
|
||||
if (m_string_builder)
|
||||
return m_string_builder->try_append(string);
|
||||
m_utf16_builder->append(Utf16String::from_utf8_without_validation(string));
|
||||
return {};
|
||||
}
|
||||
|
||||
ErrorOr<void> FormatBuilder::append(Utf16View const& string)
|
||||
{
|
||||
if (m_string_builder)
|
||||
return m_string_builder->try_append(string);
|
||||
m_utf16_builder->append(string);
|
||||
return {};
|
||||
}
|
||||
|
||||
ErrorOr<void> FormatBuilder::put_padding(char fill, size_t amount)
|
||||
{
|
||||
for (size_t i = 0; i < amount; ++i)
|
||||
TRY(m_builder.try_append(fill));
|
||||
TRY(append(fill));
|
||||
return {};
|
||||
}
|
||||
ErrorOr<void> FormatBuilder::put_literal(StringView value)
|
||||
{
|
||||
for (size_t i = 0; i < value.length(); ++i) {
|
||||
TRY(m_builder.try_append(value[i]));
|
||||
TRY(append(value[i]));
|
||||
if (value[i] == '{' || value[i] == '}')
|
||||
++i;
|
||||
}
|
||||
|
|
@ -247,22 +271,28 @@ ErrorOr<void> FormatBuilder::put_string(
|
|||
value = value.substring_view(0, used_by_string);
|
||||
|
||||
if (align == Align::Left || align == Align::Default) {
|
||||
TRY(m_builder.try_append(value));
|
||||
TRY(append(value));
|
||||
TRY(put_padding(fill, used_by_padding));
|
||||
} else if (align == Align::Center) {
|
||||
auto const used_by_left_padding = used_by_padding / 2;
|
||||
auto const used_by_right_padding = ceil_div<size_t, size_t>(used_by_padding, 2);
|
||||
|
||||
TRY(put_padding(fill, used_by_left_padding));
|
||||
TRY(m_builder.try_append(value));
|
||||
TRY(append(value));
|
||||
TRY(put_padding(fill, used_by_right_padding));
|
||||
} else if (align == Align::Right) {
|
||||
TRY(put_padding(fill, used_by_padding));
|
||||
TRY(m_builder.try_append(value));
|
||||
TRY(append(value));
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
ErrorOr<void> FormatBuilder::put_string(Utf16View const& value)
|
||||
{
|
||||
TRY(append(value));
|
||||
return {};
|
||||
}
|
||||
|
||||
ErrorOr<void> FormatBuilder::put_u64(
|
||||
u64 value,
|
||||
u8 base,
|
||||
|
|
@ -307,25 +337,25 @@ ErrorOr<void> FormatBuilder::put_u64(
|
|||
|
||||
auto const put_prefix = [&]() -> ErrorOr<void> {
|
||||
if (is_negative)
|
||||
TRY(m_builder.try_append('-'));
|
||||
TRY(append('-'));
|
||||
else if (sign_mode == SignMode::Always)
|
||||
TRY(m_builder.try_append('+'));
|
||||
TRY(append('+'));
|
||||
else if (sign_mode == SignMode::Reserved)
|
||||
TRY(m_builder.try_append(' '));
|
||||
TRY(append(' '));
|
||||
|
||||
if (prefix) {
|
||||
if (base == 2) {
|
||||
if (upper_case)
|
||||
TRY(m_builder.try_append("0B"sv));
|
||||
TRY(append("0B"sv));
|
||||
else
|
||||
TRY(m_builder.try_append("0b"sv));
|
||||
TRY(append("0b"sv));
|
||||
} else if (base == 8) {
|
||||
TRY(m_builder.try_append("0"sv));
|
||||
TRY(append("0"sv));
|
||||
} else if (base == 16) {
|
||||
if (upper_case)
|
||||
TRY(m_builder.try_append("0X"sv));
|
||||
TRY(append("0X"sv));
|
||||
else
|
||||
TRY(m_builder.try_append("0x"sv));
|
||||
TRY(append("0x"sv));
|
||||
}
|
||||
}
|
||||
return {};
|
||||
|
|
@ -333,7 +363,7 @@ ErrorOr<void> FormatBuilder::put_u64(
|
|||
|
||||
auto const put_digits = [&]() -> ErrorOr<void> {
|
||||
for (size_t i = 0; i < used_by_digits; ++i)
|
||||
TRY(m_builder.try_append(buffer[i]));
|
||||
TRY(append(buffer[i]));
|
||||
return {};
|
||||
};
|
||||
|
||||
|
|
@ -795,7 +825,7 @@ ErrorOr<void> FormatBuilder::put_hexdump(ReadonlyBytes bytes, size_t width, char
|
|||
TRY(put_padding(fill, 4));
|
||||
for (size_t j = i - min(i, width); j < i; ++j) {
|
||||
auto ch = bytes[j];
|
||||
TRY(m_builder.try_append(ch >= 32 && ch <= 127 ? ch : '.')); // silly hack
|
||||
TRY(append(ch >= 32 && ch <= 127 ? ch : '.')); // silly hack
|
||||
}
|
||||
return {};
|
||||
};
|
||||
|
|
@ -827,9 +857,10 @@ ErrorOr<void> vformat(StringBuilder& builder, StringView fmtstr, TypeErasedForma
|
|||
|
||||
ErrorOr<void> vformat(Utf16StringBuilder& builder, StringView fmtstr, TypeErasedFormatParams& params)
|
||||
{
|
||||
StringBuilder string_builder(StringBuilder::Mode::UTF16);
|
||||
TRY(vformat(string_builder, fmtstr, params));
|
||||
builder.append(string_builder.to_utf16_string());
|
||||
FormatBuilder fmtbuilder { builder };
|
||||
FormatParser parser { fmtstr };
|
||||
|
||||
TRY(vformat_impl(params, fmtbuilder, parser));
|
||||
return {};
|
||||
}
|
||||
|
||||
|
|
|
|||
24
AK/Format.h
24
AK/Format.h
|
|
@ -217,7 +217,12 @@ public:
|
|||
};
|
||||
|
||||
explicit FormatBuilder(StringBuilder& builder)
|
||||
: m_builder(builder)
|
||||
: m_string_builder(&builder)
|
||||
{
|
||||
}
|
||||
|
||||
explicit FormatBuilder(Utf16StringBuilder& builder)
|
||||
: m_utf16_builder(&builder)
|
||||
{
|
||||
}
|
||||
|
||||
|
|
@ -231,6 +236,7 @@ public:
|
|||
size_t min_width = 0,
|
||||
size_t max_width = NumericLimits<size_t>::max(),
|
||||
char fill = ' ');
|
||||
ErrorOr<void> put_string(Utf16View const& value);
|
||||
|
||||
ErrorOr<void> put_u64(
|
||||
u64 value,
|
||||
|
|
@ -306,12 +312,22 @@ public:
|
|||
|
||||
StringBuilder const& builder() const
|
||||
{
|
||||
return m_builder;
|
||||
VERIFY(m_string_builder);
|
||||
return *m_string_builder;
|
||||
}
|
||||
StringBuilder& builder()
|
||||
{
|
||||
VERIFY(m_string_builder);
|
||||
return *m_string_builder;
|
||||
}
|
||||
StringBuilder& builder() { return m_builder; }
|
||||
|
||||
private:
|
||||
StringBuilder& m_builder;
|
||||
ErrorOr<void> append(char);
|
||||
ErrorOr<void> append(StringView);
|
||||
ErrorOr<void> append(Utf16View const&);
|
||||
|
||||
StringBuilder* m_string_builder { nullptr };
|
||||
Utf16StringBuilder* m_utf16_builder { nullptr };
|
||||
|
||||
ErrorOr<void> put_f64_with_precision(
|
||||
double value,
|
||||
|
|
|
|||
|
|
@ -703,9 +703,9 @@ ErrorOr<String> UnixDateTime::to_string(StringView format, LocalTime local_time)
|
|||
|
||||
Utf16String UnixDateTime::to_utf16_string(StringView format, LocalTime local_time) const
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
StringBuilder builder;
|
||||
MUST(to_string_impl(builder, format, local_time));
|
||||
return builder.to_utf16_string();
|
||||
return Utf16String::from_utf8_without_validation(MUST(builder.to_string()));
|
||||
}
|
||||
|
||||
ByteString UnixDateTime::to_byte_string(StringView format, LocalTime local_time) const
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@ Utf16String Utf16String::from_utf8_with_replacement_character(StringView utf8_st
|
|||
if (utf8_view.validate(AllowLonelySurrogates::No))
|
||||
return Utf16String::from_utf8_without_validation(utf8_string);
|
||||
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
Utf16StringBuilder builder;
|
||||
|
||||
for (auto code_point : utf8_view) {
|
||||
if (is_unicode_surrogate(code_point))
|
||||
|
|
@ -34,7 +34,7 @@ Utf16String Utf16String::from_utf8_with_replacement_character(StringView utf8_st
|
|||
builder.append_code_point(code_point);
|
||||
}
|
||||
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
Utf16String Utf16String::from_ascii_without_validation(ReadonlyBytes ascii_string)
|
||||
|
|
@ -158,9 +158,9 @@ Utf16String Utf16String::repeated(u32 code_point, size_t count)
|
|||
code_units[length_in_code_units++] = code_unit;
|
||||
});
|
||||
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
Utf16StringBuilder builder;
|
||||
builder.append_repeated({ code_units.data(), length_in_code_units }, count);
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
Utf16String Utf16String::to_well_formed() const
|
||||
|
|
@ -180,7 +180,7 @@ String Utf16String::to_well_formed_utf8() const
|
|||
ErrorOr<void> Formatter<Utf16String>::format(FormatBuilder& builder, Utf16String const& utf16_string)
|
||||
{
|
||||
if (utf16_string.has_long_utf16_storage())
|
||||
return builder.builder().try_append(utf16_string.utf16_view());
|
||||
return builder.put_string(utf16_string.utf16_view());
|
||||
return builder.put_string(utf16_string.ascii_view());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -134,10 +134,16 @@ public:
|
|||
template<class SeparatorType, class CollectionType>
|
||||
ALWAYS_INLINE static Utf16String join(SeparatorType const& separator, CollectionType const& collection, StringView format = "{}"sv)
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
builder.join(separator, collection, format);
|
||||
Utf16StringBuilder builder;
|
||||
bool first = true;
|
||||
for (auto& item : collection) {
|
||||
if (!first)
|
||||
builder.appendff("{}", separator);
|
||||
builder.appendff(format, item);
|
||||
first = false;
|
||||
}
|
||||
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
static Utf16String repeated(u32 code_point, size_t count);
|
||||
|
|
|
|||
|
|
@ -59,27 +59,27 @@ ErrorOr<ByteString> Utf16View::to_byte_string(AllowLonelySurrogates allow_lonely
|
|||
|
||||
Utf16String Utf16View::to_ascii_lowercase() const
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units());
|
||||
Utf16StringBuilder builder(length_in_code_units());
|
||||
|
||||
for (size_t i = 0; i < length_in_code_units(); ++i)
|
||||
builder.append_code_unit(AK::to_ascii_lowercase(code_unit_at(i)));
|
||||
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
Utf16String Utf16View::to_ascii_uppercase() const
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units());
|
||||
Utf16StringBuilder builder(length_in_code_units());
|
||||
|
||||
for (size_t i = 0; i < length_in_code_units(); ++i)
|
||||
builder.append_code_unit(AK::to_ascii_uppercase(code_unit_at(i)));
|
||||
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
Utf16String Utf16View::to_ascii_titlecase() const
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units());
|
||||
Utf16StringBuilder builder(length_in_code_units());
|
||||
bool next_is_upper = true;
|
||||
|
||||
for (size_t i = 0; i < length_in_code_units(); ++i) {
|
||||
|
|
@ -93,7 +93,7 @@ Utf16String Utf16View::to_ascii_titlecase() const
|
|||
next_is_upper = code_unit == u' ';
|
||||
}
|
||||
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
Utf16String Utf16View::replace(char16_t needle, Utf16View const& replacement, ReplaceMode replace_mode) const
|
||||
|
|
@ -106,7 +106,7 @@ Utf16String Utf16View::replace(Utf16View const& needle, Utf16View const& replace
|
|||
if (is_empty())
|
||||
return {};
|
||||
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units());
|
||||
Utf16StringBuilder builder(length_in_code_units());
|
||||
auto remaining = *this;
|
||||
|
||||
do {
|
||||
|
|
@ -122,12 +122,12 @@ Utf16String Utf16View::replace(Utf16View const& needle, Utf16View const& replace
|
|||
} while (replace_mode == ReplaceMode::All && !remaining.is_empty());
|
||||
|
||||
builder.append(remaining);
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
Utf16String Utf16View::escape_html_entities() const
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16, length_in_code_units());
|
||||
Utf16StringBuilder builder(length_in_code_units());
|
||||
|
||||
for (auto code_point : *this) {
|
||||
if (code_point == '<')
|
||||
|
|
@ -142,7 +142,7 @@ Utf16String Utf16View::escape_html_entities() const
|
|||
builder.append_code_point(code_point);
|
||||
}
|
||||
|
||||
return builder.to_utf16_string();
|
||||
return builder.to_string();
|
||||
}
|
||||
|
||||
bool Utf16View::is_ascii() const
|
||||
|
|
|
|||
|
|
@ -687,7 +687,7 @@ template<>
|
|||
struct Formatter<Utf16View> : Formatter<FormatString> {
|
||||
ErrorOr<void> format(FormatBuilder& builder, Utf16View const& value)
|
||||
{
|
||||
return builder.builder().try_append(value);
|
||||
return builder.put_string(value);
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -354,7 +354,7 @@ TEST_CASE(repeated)
|
|||
|
||||
TEST_CASE(from_string_builder)
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
Utf16StringBuilder builder;
|
||||
builder.append_code_point('a');
|
||||
builder.append_code_point('b');
|
||||
builder.append_code_point(0x1f600);
|
||||
|
|
@ -363,7 +363,7 @@ TEST_CASE(from_string_builder)
|
|||
builder.append_code_point('c');
|
||||
builder.append_code_point('d');
|
||||
|
||||
auto string = builder.to_utf16_string();
|
||||
auto string = builder.to_string();
|
||||
EXPECT_EQ(string.length_in_code_units(), 10uz);
|
||||
EXPECT_EQ(string.length_in_code_points(), 7uz);
|
||||
EXPECT_EQ(string, "ab😀𐀀🍕cd"sv);
|
||||
|
|
@ -371,8 +371,8 @@ TEST_CASE(from_string_builder)
|
|||
|
||||
TEST_CASE(from_string_builder_alignment)
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
builder.append("\u00a0"sv);
|
||||
Utf16StringBuilder builder;
|
||||
builder.append(Utf16String::from_utf8_without_validation("\u00a0"sv));
|
||||
builder.append(R"~~(
|
||||
<script>
|
||||
const containsValidURL = input => {
|
||||
|
|
@ -381,7 +381,7 @@ TEST_CASE(from_string_builder_alignment)
|
|||
</script>
|
||||
)~~"sv);
|
||||
|
||||
auto string1 = builder.to_utf16_string();
|
||||
auto string1 = builder.to_string();
|
||||
auto string2 = string1.to_utf8();
|
||||
|
||||
EXPECT_EQ(string1.code_unit_at(0), 0x00a0);
|
||||
|
|
@ -415,10 +415,7 @@ TEST_CASE(from_ipc_stream)
|
|||
{
|
||||
auto data = u"hello 😀 there!"sv;
|
||||
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
builder.append(data);
|
||||
|
||||
auto buffer = MUST(builder.to_byte_buffer());
|
||||
auto buffer = MUST(ByteBuffer::copy({ reinterpret_cast<u8 const*>(data.utf16_span().data()), data.length_in_code_units() * sizeof(char16_t) }));
|
||||
FixedMemoryStream stream { buffer.bytes() };
|
||||
|
||||
auto string = TRY_OR_FAIL(Utf16String::from_ipc_stream(stream, data.length_in_code_units(), false));
|
||||
|
|
@ -438,10 +435,7 @@ TEST_CASE(from_ipc_stream)
|
|||
{
|
||||
auto data = u"😀"sv;
|
||||
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
builder.append(data);
|
||||
|
||||
auto buffer = MUST(builder.to_byte_buffer());
|
||||
auto buffer = MUST(ByteBuffer::copy({ reinterpret_cast<u8 const*>(data.utf16_span().data()), data.length_in_code_units() * sizeof(char16_t) }));
|
||||
FixedMemoryStream stream { buffer.bytes() };
|
||||
|
||||
auto result = Utf16String::from_ipc_stream(stream, data.length_in_code_units(), true);
|
||||
|
|
@ -450,10 +444,7 @@ TEST_CASE(from_ipc_stream)
|
|||
{
|
||||
auto data = u"hello 😀 there!"sv;
|
||||
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
builder.append(data);
|
||||
|
||||
auto buffer = MUST(builder.to_byte_buffer());
|
||||
auto buffer = MUST(ByteBuffer::copy({ reinterpret_cast<u8 const*>(data.utf16_span().data()), data.length_in_code_units() * sizeof(char16_t) }));
|
||||
FixedMemoryStream stream { buffer.bytes() };
|
||||
|
||||
auto result = Utf16String::from_ipc_stream(stream, data.length_in_code_units(), true);
|
||||
|
|
@ -1251,17 +1242,16 @@ TEST_CASE(optional)
|
|||
EXPECT_EQ(released, u"well 😀 hello"sv);
|
||||
}
|
||||
|
||||
TEST_CASE(utf16_builder_clear_resets_ascii_flag)
|
||||
TEST_CASE(utf16_string_builder_clear_resets_ascii_flag)
|
||||
{
|
||||
// Regression test: StringBuilder(UTF16) must reset m_utf16_builder_is_ascii
|
||||
// on clear(). Without this, a builder that previously held non-ASCII content
|
||||
// stores subsequent ASCII as char16_t, and to_utf16_string() corrupts the
|
||||
// first code unit via placement-new overlap in from_string_builder().
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
// Regression test: Utf16StringBuilder must reset its ASCII state on clear().
|
||||
// Without this, a builder that previously held non-ASCII content stores
|
||||
// subsequent ASCII as char16_t, corrupting the first code unit when adopted.
|
||||
Utf16StringBuilder builder;
|
||||
|
||||
// 1. Append a non-ASCII code point to force m_utf16_builder_is_ascii = false.
|
||||
builder.append_code_point(0x00D7); // U+00D7 MULTIPLICATION SIGN (×)
|
||||
auto first = builder.to_utf16_string();
|
||||
auto first = builder.to_string();
|
||||
EXPECT_EQ(first.length_in_code_units(), 1u);
|
||||
EXPECT_EQ(first.code_unit_at(0), 0x00D7);
|
||||
|
||||
|
|
@ -1271,7 +1261,7 @@ TEST_CASE(utf16_builder_clear_resets_ascii_flag)
|
|||
for (int i = 0; i < 100; ++i)
|
||||
builder.append_code_point(' ');
|
||||
|
||||
auto second = builder.to_utf16_string();
|
||||
auto second = builder.to_string();
|
||||
EXPECT_EQ(second.length_in_code_units(), 101u);
|
||||
EXPECT_EQ(second.code_unit_at(0), 0x000A); // Must be newline, not null.
|
||||
EXPECT_EQ(second.code_unit_at(1), 0x0020);
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
*/
|
||||
|
||||
#include <AK/StringBuilder.h>
|
||||
#include <AK/Utf16StringBuilder.h>
|
||||
#include <LibURL/Parser.h>
|
||||
#include <UI/Qt/StringUtils.h>
|
||||
|
||||
|
|
@ -44,9 +45,9 @@ QString qstring_from_utf16_string(Utf16View const& string)
|
|||
|
||||
QString vqformatted(StringView format, AK::TypeErasedFormatParams& parameters)
|
||||
{
|
||||
StringBuilder builder(StringBuilder::Mode::UTF16);
|
||||
Utf16StringBuilder builder;
|
||||
MUST(vformat(builder, format, parameters));
|
||||
return qstring_from_utf16_string(builder.utf16_string_view());
|
||||
return qstring_from_utf16_string(builder.view());
|
||||
}
|
||||
|
||||
QByteArray qbytearray_from_ak_string(StringView ak_string)
|
||||
|
|
|
|||
Loading…
Reference in a new issue