AK+Format: Exclude prefix from width calculation.

When we write the format specifier '{:#08x}' we are asking for eight
significant digits, zero padding and the prefix '0x'.

However, previously we got only six significant digits because the
prefix counted towards the width. (The number '8' here is the total
width and not the number of significant digits.)

Both fmtlib and printf shared this behaviour. However, I am introducing
a special case here because when we do zero padding we really only care
about the digits and not the width.

Notice that zero padding is a special case anyways, because zero padding
goes after the prefix as opposed to any other padding which goes before
it.

AK/Format.cpp

@@ -281,18 +281,30 @@ void FormatBuilder::put_u64(
     SignMode sign_mode,
     bool is_negative)
 {
+    if (align == Align::Default)
+        align = Align::Right;
+
     Array<u8, 128> buffer;
 
     const auto used_by_digits = convert_unsigned_to_string(value, buffer, base, upper_case);
 
-    auto used_by_prefix = sign_mode == SignMode::OnlyIfNeeded ? static_cast<size_t>(is_negative) : 1;
-    if (prefix) {
-        if (base == 8)
+    size_t used_by_prefix = 0;
+    if (align == Align::Right && zero_pad) {
+        // We want String::formatted("{:#08x}", 32) to produce '0x00000020' instead of '0x000020'. This
+        // behaviour differs from both fmtlib and printf, but is more intuitive.
+        used_by_prefix = 0;
+    } else {
+        if (is_negative || sign_mode != SignMode::OnlyIfNeeded)
             used_by_prefix += 1;
-        else if (base == 16)
-            used_by_prefix += 2;
-        else if (base == 2)
-            used_by_prefix += 2;
+
+        if (prefix) {
+            if (base == 8)
+                used_by_prefix += 1;
+            else if (base == 16)
+                used_by_prefix += 2;
+            else if (base == 2)
+                used_by_prefix += 2;
+        }
     }
 
     const auto used_by_field = used_by_prefix + used_by_digits;
@@ -341,7 +353,7 @@ void FormatBuilder::put_u64(
         put_prefix();
         put_digits();
         put_padding(fill, used_by_right_padding);
-    } else if (align == Align::Right || align == Align::Default) {
+    } else if (align == Align::Right) {
         const auto used_by_left_padding = used_by_padding;
 
         if (zero_pad) {
@@ -505,7 +517,7 @@ void Formatter<T, typename EnableIf<IsIntegral<T>::value>::Type>::format(TypeEra
 
         m_mode = Mode::Hexadecimal;
         m_alternative_form = true;
-        m_width = 2 * sizeof(void*) + 2;
+        m_width = 2 * sizeof(void*);
         m_zero_pad = true;
     }
 

AK/Format.h

@@ -204,8 +204,9 @@ private:
     Array<TypeErasedParameter, sizeof...(Parameters)> m_data;
 };
 
-// We use the same format for most types for consistency. This is taken directly from std::format.
-// Not all valid options do anything yet.
+// We use the same format for most types for consistency. This is taken directly from
+// std::format. One difference is that we are not counting the width or sign towards the
+// total width when calculating zero padding for numbers.
 // https://en.cppreference.com/w/cpp/utility/format/formatter#Standard_format_specification
 struct StandardFormatter {
     enum class Mode {

AK/Tests/TestFormat.cpp

@@ -111,6 +111,8 @@ TEST_CASE(zero_pad)
     EXPECT_EQ(String::formatted("{: <010}", 42), "42        ");
     EXPECT_EQ(String::formatted("{:010}", 42), "0000000042");
     EXPECT_EQ(String::formatted("{:/^010}", 42), "////42////");
+    EXPECT_EQ(String::formatted("{:04x}", -32), "-0020");
+    EXPECT_EQ(String::formatted("{:#06x}", -64), "-0x000040");
 }
 
 TEST_CASE(replacement_field)
@@ -144,7 +146,7 @@ TEST_CASE(boolean_values)
     EXPECT_EQ(String::formatted("{:>4}", false), "false");
     EXPECT_EQ(String::formatted("{:d}", false), "0");
     EXPECT_EQ(String::formatted("{:d}", true), "1");
-    EXPECT_EQ(String::formatted("{:#08x}", true), "0x000001");
+    EXPECT_EQ(String::formatted("{:#08x}", true), "0x00000001");
 }
 
 TEST_CASE(pointers)

Applications/HexEditor/HexEditor.cpp

@@ -164,7 +164,7 @@ bool HexEditor::copy_selected_hex_to_clipboard_as_c_code()
     output_string_builder.appendff("unsigned char raw_data[{}] = {{\n", (m_selection_end - m_selection_start) + 1);
     output_string_builder.append("    ");
     for (int i = m_selection_start, j = 1; i <= m_selection_end; i++, j++) {
-        output_string_builder.appendff("{:#04X}", m_buffer.data()[i]);
+        output_string_builder.appendff("{:#02X}", m_buffer.data()[i]);
         if (i != m_selection_end)
             output_string_builder.append(", ");
         if ((j % 12) == 0) {
@@ -516,7 +516,7 @@ void HexEditor::paint_event(GUI::PaintEvent& event)
         };
 
         bool is_current_line = (m_position / bytes_per_row()) == i;
-        auto line = String::formatted("{:#010X}", i * bytes_per_row());
+        auto line = String::formatted("{:#08X}", i * bytes_per_row());
         painter.draw_text(
             side_offset_rect,
             line,

Applications/HexEditor/HexEditorWidget.cpp

@@ -54,7 +54,7 @@ HexEditorWidget::HexEditorWidget()
     m_editor = add<HexEditor>();
 
     m_editor->on_status_change = [this](int position, HexEditor::EditMode edit_mode, int selection_start, int selection_end) {
-        m_statusbar->set_text(0, String::formatted("Offset: {:#010X}", position));
+        m_statusbar->set_text(0, String::formatted("Offset: {:#08X}", position));
         m_statusbar->set_text(1, String::formatted("Edit Mode: {}", edit_mode == HexEditor::EditMode::Hex ? "Hex" : "Text"));
         m_statusbar->set_text(2, String::formatted("Selection Start: {}", selection_start));
         m_statusbar->set_text(3, String::formatted("Selection End: {}", selection_end));