Commit 27adb82fda

LemonBoy <thatlemon@gmail.com>
2020-09-17 00:41:26
std: Respect user-specified alignment when formatting ints
This implementation tries to do the right thing (TM) by treating the sign as part of the number itself, therefore the alignment parameter applies to both the sign and the digits. In other words the format string `{:>4}` with -1 as input will not output `- 1` but ` -1`. And let's default to right alignment for everything as that's what users want, especially when printing numbers. Many implementations use different defaults for numeric vs non-numeric types, let's strive for a consistent behaviour here.
master
1 parent bb9a4ad
Changed files (1)
lib
std
fmt.zig
lib/std/fmt.zig
@@ -22,7 +22,7 @@ pub const Alignment = enum {
 pub const FormatOptions = struct {
     precision: ?usize = null,
     width: ?usize = null,
-    alignment: Alignment = .Left,
+    alignment: Alignment = .Right,
     fill: u8 = ' ',
 };
 
@@ -631,26 +631,22 @@ pub fn formatBuf(
     writer: anytype,
 ) !void {
     const width = options.width orelse buf.len;
-    var padding = if (width > buf.len) (width - buf.len) else 0;
-    const pad_byte = [1]u8{options.fill};
+    const padding = if (width > buf.len) (width - buf.len) else 0;
+
     switch (options.alignment) {
         .Left => {
             try writer.writeAll(buf);
-            while (padding > 0) : (padding -= 1) {
-                try writer.writeAll(&pad_byte);
-            }
+            try writer.writeByteNTimes(options.fill, padding);
         },
         .Center => {
-            const padl = padding / 2;
-            var i: usize = 0;
-            while (i < padl) : (i += 1) try writer.writeAll(&pad_byte);
+            const left_padding = padding / 2;
+            const right_padding = (padding + 1) / 2;
+            try writer.writeByteNTimes(options.fill, left_padding);
             try writer.writeAll(buf);
-            while (i < padding) : (i += 1) try writer.writeAll(&pad_byte);
+            try writer.writeByteNTimes(options.fill, right_padding);
         },
         .Right => {
-            while (padding > 0) : (padding -= 1) {
-                try writer.writeAll(&pad_byte);
-            }
+            try writer.writeByteNTimes(options.fill, padding);
             try writer.writeAll(buf);
         },
     }
@@ -941,61 +937,27 @@ pub fn formatInt(
     options: FormatOptions,
     writer: anytype,
 ) !void {
+    assert(base >= 2);
+
     const int_value = if (@TypeOf(value) == comptime_int) blk: {
         const Int = math.IntFittingRange(value, value);
         break :blk @as(Int, value);
     } else
         value;
 
-    if (@typeInfo(@TypeOf(int_value)).Int.is_signed) {
-        return formatIntSigned(int_value, base, uppercase, options, writer);
-    } else {
-        return formatIntUnsigned(int_value, base, uppercase, options, writer);
-    }
-}
+    const value_info = @typeInfo(@TypeOf(int_value)).Int;
 
-fn formatIntSigned(
-    value: anytype,
-    base: u8,
-    uppercase: bool,
-    options: FormatOptions,
-    writer: anytype,
-) !void {
-    const new_options = FormatOptions{
-        .width = if (options.width) |w| (if (w == 0) 0 else w - 1) else null,
-        .precision = options.precision,
-        .fill = options.fill,
-    };
-    const bit_count = @typeInfo(@TypeOf(value)).Int.bits;
-    const Uint = std.meta.Int(false, bit_count);
-    if (value < 0) {
-        try writer.writeAll("-");
-        const new_value = math.absCast(value);
-        return formatIntUnsigned(new_value, base, uppercase, new_options, writer);
-    } else if (options.width == null or options.width.? == 0) {
-        return formatIntUnsigned(@intCast(Uint, value), base, uppercase, options, writer);
-    } else {
-        try writer.writeAll("+");
-        const new_value = @intCast(Uint, value);
-        return formatIntUnsigned(new_value, base, uppercase, new_options, writer);
-    }
-}
+    // The type must have the same size as `base` or be wider in order for the
+    // division to work
+    const min_int_bits = comptime math.max(value_info.bits, 8);
+    const MinInt = std.meta.Int(false, min_int_bits);
 
-fn formatIntUnsigned(
-    value: anytype,
-    base: u8,
-    uppercase: bool,
-    options: FormatOptions,
-    writer: anytype,
-) !void {
-    assert(base >= 2);
-    const value_info = @typeInfo(@TypeOf(value)).Int;
-    var buf: [math.max(value_info.bits, 1)]u8 = undefined;
-    const min_int_bits = comptime math.max(value_info.bits, @typeInfo(@TypeOf(base)).Int.bits);
-    const MinInt = std.meta.Int(value_info.is_signed, min_int_bits);
-    var a: MinInt = value;
-    var index: usize = buf.len;
+    const abs_value = math.absCast(int_value);
+    // The worst case in terms of space needed is base 2, plus 1 for the sign
+    var buf: [1 + math.max(value_info.bits, 1)]u8 = undefined;
 
+    var a: MinInt = abs_value;
+    var index: usize = buf.len;
     while (true) {
         const digit = a % base;
         index -= 1;
@@ -1004,25 +966,21 @@ fn formatIntUnsigned(
         if (a == 0) break;
     }
 
-    const digits_buf = buf[index..];
-    const width = options.width orelse 0;
-    const padding = if (width > digits_buf.len) (width - digits_buf.len) else 0;
-
-    if (padding > index) {
-        const zero_byte: u8 = options.fill;
-        var leftover_padding = padding - index;
-        while (true) {
-            try writer.writeAll(@as(*const [1]u8, &zero_byte)[0..]);
-            leftover_padding -= 1;
-            if (leftover_padding == 0) break;
+    if (value_info.is_signed) {
+        if (value < 0) {
+            // Negative integer
+            index -= 1;
+            buf[index] = '-';
+        } else if (options.width == null or options.width.? == 0) {
+            // Positive integer, omit the plus sign
+        } else {
+            // Positive integer
+            index -= 1;
+            buf[index] = '+';
         }
-        mem.set(u8, buf[0..index], options.fill);
-        return writer.writeAll(&buf);
-    } else {
-        const padded_buf = buf[index - padding ..];
-        mem.set(u8, padded_buf[0..padding], options.fill);
-        return writer.writeAll(padded_buf);
     }
+
+    return formatBuf(buf[index..], options, writer);
 }
 
 pub fn formatIntBuf(out_buf: []u8, value: anytype, base: u8, uppercase: bool, options: FormatOptions) usize {
@@ -1287,7 +1245,17 @@ test "int.specifier" {
 
 test "int.padded" {
     try testFmt("u8: '   1'", "u8: '{:4}'", .{@as(u8, 1)});
-    try testFmt("u8: 'xxx1'", "u8: '{:x<4}'", .{@as(u8, 1)});
+    try testFmt("u8: '1000'", "u8: '{:0<4}'", .{@as(u8, 1)});
+    try testFmt("u8: '0001'", "u8: '{:0>4}'", .{@as(u8, 1)});
+    try testFmt("u8: '0100'", "u8: '{:0^4}'", .{@as(u8, 1)});
+    try testFmt("i8: '-1  '", "i8: '{:<4}'", .{@as(i8, -1)});
+    try testFmt("i8: '  -1'", "i8: '{:>4}'", .{@as(i8, -1)});
+    try testFmt("i8: ' -1 '", "i8: '{:^4}'", .{@as(i8, -1)});
+    try testFmt("i16: '-1234'", "i16: '{:4}'", .{@as(i16, -1234)});
+    try testFmt("i16: '+1234'", "i16: '{:4}'", .{@as(i16, 1234)});
+    try testFmt("i16: '-12345'", "i16: '{:4}'", .{@as(i16, -12345)});
+    try testFmt("i16: '+12345'", "i16: '{:4}'", .{@as(i16, 12345)});
+    try testFmt("u16: '12345'", "u16: '{:4}'", .{@as(u16, 12345)});
 }
 
 test "buffer" {
@@ -1333,7 +1301,7 @@ test "slice" {
         try testFmt("slice: []const u8@deadbeef\n", "slice: {}\n", .{value});
     }
 
-    try testFmt("buf: Test \n", "buf: {s:5}\n", .{"Test"});
+    try testFmt("buf:  Test\n", "buf: {s:5}\n", .{"Test"});
     try testFmt("buf: Test\n Other text", "buf: {s}\n Other text", .{"Test"});
 }
 
@@ -1366,7 +1334,7 @@ test "cstr" {
         .{@ptrCast([*c]const u8, "Test C")},
     );
     try testFmt(
-        "cstr: Test C    \n",
+        "cstr:     Test C\n",
         "cstr: {s:10}\n",
         .{@ptrCast([*c]const u8, "Test C")},
     );
@@ -1809,7 +1777,7 @@ test "vector" {
 
     try testFmt("{ true, false, true, false }", "{}", .{vbool});
     try testFmt("{ -2, -1, 0, 1 }", "{}", .{vi64});
-    try testFmt("{ -   2, -   1, +   0, +   1 }", "{d:5}", .{vi64});
+    try testFmt("{    -2,    -1,    +0,    +1 }", "{d:5}", .{vi64});
     try testFmt("{ 1000, 2000, 3000, 4000 }", "{}", .{vu64});
     try testFmt("{ 3e8, 7d0, bb8, fa0 }", "{x}", .{vu64});
     try testFmt("{ 1kB, 2kB, 3kB, 4kB }", "{B}", .{vu64});
@@ -1822,15 +1790,16 @@ test "enum-literal" {
 
 test "padding" {
     try testFmt("Simple", "{}", .{"Simple"});
-    try testFmt("true      ", "{:10}", .{true});
+    try testFmt("      true", "{:10}", .{true});
     try testFmt("      true", "{:>10}", .{true});
     try testFmt("======true", "{:=>10}", .{true});
     try testFmt("true======", "{:=<10}", .{true});
     try testFmt("   true   ", "{:^10}", .{true});
     try testFmt("===true===", "{:=^10}", .{true});
-    try testFmt("Minimum            width", "{:18} width", .{"Minimum"});
+    try testFmt("           Minimum width", "{:18} width", .{"Minimum"});
     try testFmt("==================Filled", "{:=>24}", .{"Filled"});
     try testFmt("        Centered        ", "{:^24}", .{"Centered"});
+    try testFmt("-", "{:-^1}", .{""});
 }
 
 test "decimal float padding" {