feat: word-phrase TimeSpan dehumanize + Humanize round-trip (#691)#1801
feat: word-phrase TimeSpan dehumanize + Humanize round-trip (#691)#1801wyf027 wants to merge 9 commits into
Conversation
Parse compact duration strings such as 3h18m, 12m 30s, 1000s, 6d, and colon forms with configurable minutes/seconds interpretation. Co-authored-by: Cursor <cursoragent@cursor.com>
Remove redundant global using, restore missing colon-format test method, pin culture with UseCulture, and return false from Try* on overflow. Co-authored-by: Cursor <cursoragent@cursor.com>
Use nullable optional default `null` and reflection member order (ctor, ColonFormat, Default) across all target frameworks. Co-authored-by: Cursor <cursoragent@cursor.com>
Colon arithmetic mishandles negative components (e.g. -3:18:00). Reject negative segments so standard TryParse handles them; add regression test. Co-authored-by: Cursor <cursoragent@cursor.com>
Skip comma and "and" separators between number-unit tokens so strings like "3 hours, 18 minutes" parse correctly. Add round-trip tests against TimeSpan.Humanize() output for common unit combinations. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces TimeSpan dehumanization—parsing human-friendly duration strings back into TimeSpan values. It adds a new enum (TimeSpanDehumanizeColonFormat), an options class, and extension methods supporting three parsing strategies: compact unit-tokens (e.g., "3h18m"), colon-separated values (e.g., "3:18" with configurable interpretation), and TimeSpan.TryParse fallback. ChangesTimeSpan Dehumanization Feature
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Humanizer/TimeSpanDehumanizeExtensions.cs (1)
207-211: 💤 Low valueConsider adding "wk" and "wks" abbreviations for weeks.
The week unit supports "w", "week", and "weeks", but common abbreviations "wk" and "wks" are missing. This would improve consistency with other units that support both short and medium abbreviations (e.g., "hr"/"hrs" for hours).
Suggested fix
- if (UnitEquals(unit, "w") || UnitEquals(unit, "week") || UnitEquals(unit, "weeks")) + if (UnitEquals(unit, "w") || UnitEquals(unit, "wk") || UnitEquals(unit, "wks") || UnitEquals(unit, "week") || UnitEquals(unit, "weeks"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Humanizer/TimeSpanDehumanizeExtensions.cs` around lines 207 - 211, The UnitEquals checks for weeks in the TimeSpanDehumanizeExtensions method only match "w", "week", and "weeks"; update the condition that sets result = TimeSpan.FromDays(7 * amount) (the block using UnitEquals and TimeSpan.FromDays) to also accept the abbreviations "wk" and "wks" so UnitEquals(unit, "wk") and UnitEquals(unit, "wks") are included alongside the existing checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Humanizer/TimeSpanDehumanizeExtensions.cs`:
- Around line 207-211: The UnitEquals checks for weeks in the
TimeSpanDehumanizeExtensions method only match "w", "week", and "weeks"; update
the condition that sets result = TimeSpan.FromDays(7 * amount) (the block using
UnitEquals and TimeSpan.FromDays) to also accept the abbreviations "wk" and
"wks" so UnitEquals(unit, "wk") and UnitEquals(unit, "wks") are included
alongside the existing checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a506516-b7a8-483b-934a-6a615fa943ed
📒 Files selected for processing (8)
src/Humanizer/TimeSpanDehumanizeColonFormat.cssrc/Humanizer/TimeSpanDehumanizeExtensions.cssrc/Humanizer/TimeSpanDehumanizeOptions.cstests/Humanizer.Tests/ApiApprover/PublicApiApprovalTest.Approve_Public_Api.DotNet10_0.verified.txttests/Humanizer.Tests/ApiApprover/PublicApiApprovalTest.Approve_Public_Api.DotNet11_0.verified.txttests/Humanizer.Tests/ApiApprover/PublicApiApprovalTest.Approve_Public_Api.DotNet8_0.verified.txttests/Humanizer.Tests/ApiApprover/PublicApiApprovalTest.Approve_Public_Api.Net4_8.verified.txttests/Humanizer.Tests/TimeSpanDehumanizeTests.cs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc1f99011f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (colonFormat == TimeSpanDehumanizeColonFormat.MinutesSeconds | ||
| && TryParseInvariantInt(parts[0], out var minutesOnly) | ||
| && TryParseInvariantDouble(parts[1], out var secondsOnly) | ||
| && minutesOnly >= 0 && secondsOnly >= 0) |
There was a problem hiding this comment.
When ColonFormat is set to MinutesSeconds, negative inputs like "-18:01" do not use that interpretation because this branch rejects any negative component (minutesOnly >= 0 && secondsOnly >= 0), and parsing then falls through to TimeSpan.TryParse, which interprets the same text as hours/minutes. This makes the option inconsistent and can silently return a duration with the wrong magnitude for valid user input.
Useful? React with 👍 / 👎.
|
Addressed the MinutesSeconds negative colon parsing review. The parser now applies the leading sign to the whole colon duration, and |
1 participant
Summary
Extends #1800 (MVP TimeSpan dehumanize) with English word-phrase parsing:
,andandseparators between number-unit tokens (3 hours, 18 minutes,3 hours and 18 minutes)TimeSpan.Humanize()output for hours/minutes/seconds/weeksDepends on
Out of scope
Humanize()(11 months, 30 days)toWords: trueoutput (three hours)Part of #691.
Made with Cursor