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

QuantityFormatter defaults to the "G" format #1450

Merged
merged 3 commits into from
Dec 15, 2024

Conversation

lipchev
Copy link
Collaborator

@lipchev lipchev commented Dec 11, 2024

  • QuantityFormatter defaults to the "G" format for null or empty string
  • QuantityFormatter no longer supports the U/V/Q formats (an exception is thrown)
  • QuantityFormatter explicitly throws for the Cx/Px formats
  • QuantityFormatter shouldn't throw for something like "P1: #.00" (instead it should output something like "P1: 12.34 mg")
  • QuantityFormatter unless explicitly given an incorrect Ax specifier, the QuantityFormatter uses the "default unit abbreviation" - which maybe string.Empty (if none are defined for a given unit)

Fixes #1183
Duplicate of #1206

CC @tmilnthorp

- QuantityFormatter no longer supports the U/V/Q formats
- QuantityFormatter explicitly throws for the Cx/Px formats
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After you've finished reviewing these, let's move them to the QuantityFormatterTests.cs - none of the tests in this file should reference the QuantityFormatter.

Comment on lines +210 to +215
private static string FormatWithValueAndAbbreviation<TUnitType>(IQuantity<TUnitType> quantity, string format, IFormatProvider formatProvider)
where TUnitType : Enum
{
var abbreviation = UnitsNetSetup.Default.UnitAbbreviations.GetDefaultAbbreviation(quantity.Unit, formatProvider);
return string.Format(formatProvider, $"{{0:{format}}} {{1}}", quantity.Value, abbreviation);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another change from the existing behavior (which I think is necessary) is that the when in the case of the "standard numeric format" (which is the default for Volume.FromAuTablespoon(1).ToString()) we again rely on the default abbreviation (possibly string.Empty) instead of throwing, as would be the case if you attempt this on a previous version (e.g. v5):

Volume.FromAuTablespoon(1).ToString("g"); // FormatException??

@lipchev
Copy link
Collaborator Author

lipchev commented Dec 15, 2024

@angularsen Eh, just wanted to note that I personally have no preference here regarding whether "G" or "S" is the default format- I just took the #1206 as a starting point.. Same goes for the exceptions. The removal of the extra formats- I'm very happy about...

@angularsen
Copy link
Owner

angularsen commented Dec 15, 2024

Eh, just wanted to note that I personally have no preference here regarding whether "G" or "S" is the default format

I think G is the better choice actually, since S may subtly hide precision they were interested in.

If they want pretty formatting they should take control instead of us trying to guess if they prefer scientific notation or fixed point. I am no longer sure the S format is a good idea, but I guess we can keep it since it is now opt-in instead of default.

@angularsen angularsen merged commit 095c85b into angularsen:release/v6 Dec 15, 2024
1 check failed
@lipchev lipchev deleted the quantity-formatter-v6 branch December 16, 2024 15:18
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.

2 participants