Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.fmt.fmtIntSizeDec() and fmtIntSizeBin() do not consider FormatOptions #9184

Closed
mikdusan opened this issue Jun 21, 2021 · 3 comments
Closed
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@mikdusan
Copy link
Member

mikdusan commented Jun 21, 2021

Ultimately formatSizeImpl() is used to implement fmtIntSizeDec() or fmtIntSizeBin() and it does not consider FormatOptions and probably should.

motivating example (from @espindola via IRC):

const std = @import("std");

pub fn main() !void {
    const stdout = std.io.getStdOut().writer();

    var buf: [32]u8 = undefined;
    var slice = buf[0..];
    const partial = try std.fmt.bufPrint(slice, "{}", .{std.fmt.fmtIntSizeDec(42 * 1000)});

    try stdout.print("X {s:<14} Y\n", .{partial});
    try stdout.print("X {:<14} Y\n", .{std.fmt.fmtIntSizeDec(42 * 1000)});
}
$ zig run foo.zig
X 42kB           Y
X 42kB Y

@andrewrk you mentioned in IRC:

it should also be renamed to remove the redundant fmt

There is a pattern in std.fmt where all pub fn fmt*() functions return a Formatter() object; a grep follows. Do you want these renamed like fmtFirstSecond()firstSecond()?

pub fn fmtSliceHexLower(bytes: []const u8) std.fmt.Formatter(formatSliceHexLower) {
pub fn fmtSliceHexUpper(bytes: []const u8) std.fmt.Formatter(formatSliceHexUpper) {
pub fn fmtSliceEscapeLower(bytes: []const u8) std.fmt.Formatter(formatSliceEscapeLower) {
pub fn fmtSliceEscapeUpper(bytes: []const u8) std.fmt.Formatter(formatSliceEscapeUpper) {
pub fn fmtIntSizeDec(value: u64) std.fmt.Formatter(formatSizeDec) {
pub fn fmtIntSizeBin(value: u64) std.fmt.Formatter(formatSizeBin) {
pub fn fmtDuration(ns: u64) Formatter(formatDuration) {
@mikdusan mikdusan added enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library. labels Jun 21, 2021
@Vexu Vexu added this to the 0.9.0 milestone Aug 6, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.11.0 Nov 24, 2021
@ghost
Copy link

ghost commented Jul 16, 2022

Another issue where it seems like a PR has been merged but the issue is still open. Can this be closed?

@Vexu
Copy link
Member

Vexu commented Jul 16, 2022

That PR did not address this issue.

@ghost
Copy link

ghost commented Jul 17, 2022

Oh, I see - that PR handled a similar issue with different functions. I'll see if I can do this one myself.

@Vexu Vexu closed this as completed in b8bf5de Jul 21, 2022
@Vexu Vexu modified the milestones: 0.12.0, 0.10.0 Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants