Skip to content

Add support for lossy format strings - #1693

Merged
djc merged 4 commits into
chronotope:mainfrom
Qelxiros:strftimeitems_lossy
Apr 11, 2025
Merged

djc merged 4 commits into
chronotope:mainfrom
Qelxiros:strftimeitems_lossy

Conversation

@Qelxiros

@Qelxiros Qelxiros commented Apr 9, 2025

Copy link
Copy Markdown
Contributor

closes #1692

@djc You described a parse_lossy function, but I ended up writing a new_lossy function. If you think it's better to set lossy at parse time, I'm happy to change it. However, it seems better to be able to set it for the Iterator directly.

@codecov

codecov Bot commented Apr 9, 2025 •

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 73.91304% with 24 lines in your changes missing coverage. Please review.

Project coverage is 90.61%. Comparing base (2c95b0a) to head (80a6532).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/format/strftime.rs 73.91% 24 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1693      +/-   ##
==========================================
- Coverage   90.67%   90.61%   -0.07%     
==========================================
  Files          38       38              
  Lines       16874    16945      +71     
==========================================
+ Hits        15301    15354      +53     
- Misses       1573     1591      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/format/strftime.rs Outdated
Comment on lines +503 to +527
macro_rules! error {
() => {
if self.lossy { Item::Literal(&original[..error_len]) } else { Item::Error }
};
($ch:expr, _) => {
if self.lossy {
error_len -= $ch.len_utf8();
remainder = &original[error_len..];
}
};
($ch:expr) => {{
error!($ch, _);
error!()
}};
}

macro_rules! return_error {
() => {
return Some((remainder, error!()))
};
($ch:expr) => {
error!($ch, _);
return_error!();
};
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, I don't want all this macro mess here -- there's probably too much already. Can you figure out a better way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The most recent commit should be good. The coverage dropped because uncovered paths that used to be one line are now several.

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's much better, thanks!

Wondering if lossy is the best name for this... maybe lenient or copy_invalid? Any thoughts/suggestions?

Comment thread src/format/strftime.rs Outdated
@Qelxiros

Copy link
Copy Markdown
Contributor Author

I agree that lossy isn't a great name, and lenient has parity with something like Java's Calendar.setLenient, so that seems like the best option. Let me know if you need anything else.

@djc

djc commented Apr 11, 2025

Copy link
Copy Markdown
Member

I don't think lenient formatting is important enough to earn API exposure on every type, please confine it to the format module.

@Qelxiros

Copy link
Copy Markdown
Contributor Author

Fixed.

@djc
djc merged commit e2bd1d1 into chronotope:main Apr 11, 2025
@Qelxiros
Qelxiros deleted the strftimeitems_lossy branch April 11, 2025 07:17
takumi-earth pushed a commit to strict-rs/strict-chrono that referenced this pull request Aug 17, 2026

This branch was previously deployed

1 inactive deployment
Coverage — 80a65321 Deployed Apr 11, 2025 by Qelxiros
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.

Feature Request: Add non-panicking infallible format functions

2 participants