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

DelayFormat now works with alignment and width #320

Merged
merged 3 commits into from Jun 25, 2019
Merged

DelayFormat now works with alignment and width #320

merged 3 commits into from Jun 25, 2019

Conversation

SamokhinIlya
Copy link
Contributor

Fix #233

@quodlibetor
Copy link
Contributor

Thanks!

Could you add a couple of tests that demonstrate the behavior and correctness?

@quodlibetor quodlibetor self-requested a review June 21, 2019 21:21
@SamokhinIlya
Copy link
Contributor Author

Sure. Also I have question about try! macro in format function. Is it used there intentionally or just hasn't been updated to question mark operator? If it's the latter, can I replace it?

@quodlibetor
Copy link
Contributor

Thanks! As you noticed, chrono still supports ancient rust that doesn't have the question mark operator.

These new tests are confusing, in them you're creating a string without alignment, and then aligning that string, which isn't related to the chrono formatting.

This does match my expectation of what would happen, since I don't think of the chrono formatter as knowing about padding.

@SamokhinIlya
Copy link
Contributor Author

SamokhinIlya commented Jun 24, 2019

These new tests are confusing, in them you're creating a string without alignment, and then aligning that string, which isn't related to the chrono formatting.

Do you mean it'll be better to inline it like this?

assert_eq!("  2007 January 02", format!("{:>17}", ymd));
assert_eq!("2007 January 02  ", format!("{:<17}", ymd));
assert_eq!(" 2007 January 02 ", format!("{:^17}", ymd));

@quodlibetor
Copy link
Contributor

Oh darn, I misunderstood what the code was doing. I saw:

        let percent = datetime.format("%%");

as

        let percent: String = datetime.format("%%");

but it's actually

       let percent: ::format::DelayedFormat<_> = datetime.format("%%");

That makes me pretty happy with this, thanks!

@quodlibetor quodlibetor merged commit 77e2770 into chronotope:master Jun 25, 2019
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.

DelayedFormat does not respect format items
2 participants