Skip to content

More std.fmt goodness #6411

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

Merged
merged 2 commits into from
Nov 25, 2020
Merged

More std.fmt goodness #6411

merged 2 commits into from
Nov 25, 2020

Conversation

LemonBoy
Copy link
Contributor

@LemonBoy LemonBoy commented Sep 24, 2020

I'm pushing this branch so you can give it a spin to make sure there are no regressions and, if possible, find an answer to a few open questions.

The good

  • comptime seems to work, stage1 didn't catch fire
  • Named arguments are here, enjoy
  • Runtime-specified width & precision are now supported
  • Thanks to the quasi-equivalence between tuples and anonymous structs (the former are anonymous structures where every field name is equal to its index) we can use the same syntax for named and positional arguments ({[NAME]} or {[POSITION]})

The bad

  • Runtime parameters means the options structure is now built at runtime, expect a (small) speed/size hit
  • More tests! (Do we need more tests?)
  • Documentation needs updating (for the love of $DEITY don't let me write docs)
  • You tell me

The ugly

  • Duplicate syntax for positionals, you can use both {N} and {[N]} for referring to the N-th parameter. Is this unacceptable and discusting? Is this fine?
    • If we're in for some (Do you use positional specifiers in your format strings?) breakage what about requiring positionals & named arguments to be enclosed in []?
  • I hope none of the supposed-to-stay-in-the-compiler machinery finds its way into the final binary
  • You

There's no specifier to tell the width/precision is taken from the following argument, do we want that?

@data-man
Copy link
Contributor

Named arguments are here, enjoy

Maybe use @ as additional feature?

try testFmt("hello world!", "{@greeting @who!}", .{ .greeting = "hello", .who = "world" });

@FireFox317
Copy link
Contributor

Maybe use @ as additional feature?

I wouldn't use @. @ should only be used for compiler builtins in my opinion.

@data-man
Copy link
Contributor

@FireFox317

@ should only be used for compiler builtins in my opinion.

and [ and ] should only be used for access by index? :)

@FireFox317
Copy link
Contributor

FireFox317 commented Sep 24, 2020

@data-man

and [ and ] should only be used for access by index? :)

I guess so? I just think we should treat compiler builtins as quite special and thus not use the @ symbol. But any way that doesn't really matter that much :) This PR is huge tho 👍

@LemonBoy
Copy link
Contributor Author

Hmm, the Windows failure looks legit... Can anyone pinpoint where (and/or on what file) it's crashing?

@marler8997
Copy link
Contributor

I feel like the description is just a test to see who will read it till the end?

@Rocknest
Copy link
Contributor

The ugly (1)(a) 'what about requiring positionals & named arguments to be enclosed in []?'

Seems like a good idea 👍🏻

@andrewrk
Copy link
Member

I don't think the issue is caused by this branch - I'm seeing a similar failure in master. I suspect 0f31113

@andrewrk
Copy link
Member

can you try rebasing to after f8b3543 ?

@LemonBoy
Copy link
Contributor Author

can you try rebasing to after f8b3543 ?

Done, but the failure's still there :(

@andrewrk
Copy link
Member

I'll help with this eventually, but I do want to focus on my branch #6250 until it's done

@ifreund
Copy link
Member

ifreund commented Nov 6, 2020

This will be very nice to have for code generation in zig-wayland.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with merging this as-is. Mind dealing with the conflict(s)?

Turn the FSM parser into a linear one so that's easier to implement new
features and/or more error checking without adding more and more states.
Functionally-speaking the two parsers are at feature parity.
@andrewrk andrewrk merged commit a06afa4 into ziglang:master Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants