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

Properly support padding in parsers #1425

Open
pitdicker opened this issue Feb 11, 2024 · 2 comments
Open

Properly support padding in parsers #1425

pitdicker opened this issue Feb 11, 2024 · 2 comments

Comments

@pitdicker
Copy link
Collaborator

With our strftime syntax it is possible to specify padding (zero padding %0?, spaces as padding %_?, no padding at all %-?, or the default).
This maps to three variants in the Pad enum : None, Zero and Space. That is good enough for formatting.

For parsing we currently just ignore whatever is specified as padding. We trim all whitespace, accept leading zero(s), and read a number.

There are cases where users want parsing to be forgiving (as it has always been in chrono), and others where users want the parser to be very strict.

I propose that in 0.5 we add at least one more variant to the Pad enum. That should allow the parser code to know if the users wants a forgiving parser (by not specifying anything about padding), or if he explicitly asks for none, zero or space padding.

Related issues: #1112, #1118. The PR that was merged in the 0.5 branch to be exact about whitespace exaggerates this issue, because with it the parser code now only accepts no padding or zero padding (#807).

@pitdicker
Copy link
Collaborator Author

pitdicker commented Feb 11, 2024

If we would add a Default variant to Pad that would mean we have to learn the formatting and parsing code what the defaults for all possible formatting fields are. That is not very elegant.

Instead we should add something like ZeroOrNone:

pub enum Pad {
    None,
    Zero,
    Space,
    ZeroOrNone,
}

Zero and Space would require padding, None would disallow padding, ZeroOrNone would allow optional zero padding.

Alternatively we can define Zero to allow optional padding and add a ZeroRequired variant.
That would be closer to the current behaviour. The only part that would make that a potentially breaking change on 0.4.x is the addition of an extra enum variant.

@djc
Copy link
Contributor

djc commented Feb 29, 2024

I feel like your Pad::ZeroOrNone should maybe be expressed as Option<Pad> instead? What does "the default" end up meaning in the formatting use case?

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

No branches or pull requests

2 participants