Add positional, precision and width support to std.fmt #2714
Conversation
| width: ?usize = null, | ||
| }; | ||
|
|
||
| fn nextArg(comptime used_pos_args: *u32, comptime maybe_pos_arg: ?comptime_int, comptime next_arg: *comptime_int) comptime_int { |
There was a problem hiding this comment.
This was part of the motivation for #2710. I initially tried to handle this function in a function called formatRaw which performed this logic, and if successful, passed the correct value through to formatType. However, this required passing varargs, hence why it is now like this.
std/fmt.zig
Outdated
| @@ -20,17 +36,28 @@ pub fn format( | |||
| comptime fmt: []const u8, | |||
| args: ..., | |||
| ) Errors!void { | |||
| if (args.len > 32) { | |||
There was a problem hiding this comment.
Arbitrary choice. Can be increased but I think this is a suitable limit for now anyway. I'm unsure you can even print 32 arguments without hitting the default comptime evaluation limit now anyway.
| Pointer, | ||
| }; | ||
|
|
||
| comptime var start_index = 0; | ||
| comptime var state = State.Start; | ||
| comptime var next_arg = 0; | ||
| comptime var maybe_pos_arg: ?comptime_int = null; |
There was a problem hiding this comment.
May be tempted to move all this state into a struct if the number of fields here grow. Still need to add a few extra features (fill, alignment etc).
| else => { | ||
| @compileError("Unexpected character in precision value: " ++ [_]u8{c}); | ||
| }, | ||
| }, | ||
| .Pointer => switch (c) { |
There was a problem hiding this comment.
I would like to move this processing into formatType but I'm missing something in regards to how types appear to change/decay when being passed through var parameters at comptime. Right now this is limited in the fact that it doesn't accept any formatting options (e.g. padding).
| start_index = i + 1; | ||
| }, | ||
| else => @compileError("Unexpected format character after '*'"), | ||
| }, | ||
| } | ||
| } | ||
| comptime { | ||
| if (args.len != next_arg) { | ||
| // All arguments must have been printed but we allow mixing positional and fixed to achieve this. |
There was a problem hiding this comment.
This is to handle intermixing positionals and non-positionals where by themselves we fail to iterate through the whole set.
| width = comptime (parseUnsigned(usize, fmt[1..], 10) catch unreachable); | ||
| } | ||
| return formatBytes(value, width, 1000, context, Errors, output); | ||
| if (comptime std.mem.eql(u8, fmt, "B")) { |
There was a problem hiding this comment.
I've changed all the format specifier checks to check the entirety of the format string and not simply the first character. Previously you could do something like {sqpweouqwe} and it would be valid, provided the first few happened to be valid.
std/fmt.zig
Outdated
| '.' => try formatFloatDecimal(value, width, context, Errors, output), | ||
| else => @compileError("Unknown format character: " ++ [_]u8{float_fmt}), | ||
| if (fmt.len == 0 or comptime std.mem.eql(u8, fmt, "e")) { | ||
| // TODO: bug: Accessing options.precision directly results in null even with a visible payload. |
There was a problem hiding this comment.
Fairly sure this is a bug. When I log the value of options.precision via a warn I get null as the result even if it has a non-null payload. I think this was also the case with a @compileLog but would need to double-check. Regardless, I would expect to be able to pass options.precision directly with no issue.
std/fmt.zig
Outdated
| @@ -763,7 +889,7 @@ fn formatIntUnsigned( | |||
| const padding = if (width > digits_buf.len) (width - digits_buf.len) else 0; | |||
|
|
|||
| if (padding > index) { | |||
| const zero_byte: u8 = '0'; | |||
| const zero_byte: u8 = ' '; | |||
There was a problem hiding this comment.
Remnant I forgot to remove. Will update.
std/fmt.zig
Outdated
| @@ -1000,27 +1126,32 @@ test "fmt.int.specifier" { | |||
| } | |||
| } | |||
|
|
|||
| test "fmt.buffer" { | |||
| test "int.padded" { | |||
| // TODO: This should pad with spaces | |||
There was a problem hiding this comment.
Again, remnant, will update.
| assert(mem.eql(u8, buf3.toSlice(), "S{ .a = S{ .a = S{ .a = S{ ... }, .tu = TU{ ... }, .e = E.Two, .vec = (10.200,2.220) }, .tu = TU{ .ptr = TU{ ... } }, .e = E.Two, .vec = (10.200,2.220) }, .tu = TU{ .ptr = TU{ .ptr = TU{ ... } } }, .e = E.Two, .vec = (10.200,2.220) }")); | ||
| } | ||
|
|
||
| test "positional" { | ||
| try testFmt("2 1 0", "{2} {1} {0}", usize(0), usize(1), usize(2)); |
There was a problem hiding this comment.
Functionality mimics that of https://doc.rust-lang.org/std/fmt/.
|
I tested |
tiehuis
force-pushed
the
fmt-overhaul
branch
2 times, most recently
from
c5c804f to
0450ddf
Compare
This removes the odd width and precision specifiers found and replacing them with the more consistent api described in #1358. Take the following example: {1:5.9} This refers to the first argument (0-indexed) in the argument list. It will be printed with a minimum width of 5 and will have a precision of 9 (if applicable). Not all types correctly use these parameters just yet. There are still some missing gaps to fill in. Fill characters and alignment have yet to be implemented.
These options are now available to use when printing, however nothing currently makes use of these.
This removes the odd width and precision specifiers found and replacing
them with the more consistent api described in #1358.
Take the following example:
This refers to the first argument (0-indexed) in the argument list. It
will be printed with a minimum width of 5 and will have a precision of 9
(if applicable).
Not all types correctly use these parameters just yet. There are still
some missing gaps to fill in. Fill characters and alignment has yet to
be implemented.
This is functional, but there are a few issues encountered that I'd like to try solve first, or create an issue and link to them before merging regardless.