-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
Add support for specifying separator between units when using TimeFormatter #459
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #459 +/- ##
===================================
Coverage 97% 97%
===================================
Files 96 96
Lines 3434 3437 +3
===================================
+ Hits 3317 3320 +3
Misses 117 117 ☔ View full report in Codecov by Sentry. |
Hi and thanks for the PR. |
Looks promising! |
Yes, this is already possible. We just need code to make
Very roughly expressed in code, this means: public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
{
var format = formattingInfo.Format;
// format.Items[0] contains the format for TimeFormatter
// format.Items[0] contains the format for ListFormatter
var fi = (FormattingInfo) formattingInfo; // This cast is safe
var formatterName = fi.Placeholder?.FormatterName ?? string.Empty;
var current = fi.CurrentValue;
// Now we have to check for a nested format.
// That is the one needed for the ListFormatter
if (format is { Length: > 0, HasNested: true })
{
current = new ListOfElementsToOutput_LikeInTheCodeYouProposed(); // must be an IList to work with ListFormatter
format.Items.RemoveAt(0); // That's the format for the TimeFormatter
fi.FormatAsChild(format, current); // format 'current' with ListFormatter
return true;
}
// other existing code we need for current = new ListOfElementsToOutput_LikeInTheCodeYouProposed();
All the details are just a question of time limitations. |
Also removes previous code for specifying separator on TimeTextInfo in favor of using nested ListFormatter to specify separator for time units.
Thanks for your detailed answer! I have made the suggested changes. I agree that the solution is much more elegant and flexible now! 👍 |
That was fast, thank you! |
Implementation of #458.
Since many languages needs to know if it is the last item/unit in the list in order to choose the correct separator, I had to change from using the
StringBuilder
to first build a list of the items/unit outputs and then combine them.