angularsen / UnitsNet

Makes life working with units of measurement just a little bit better.
https://www.nuget.org/packages/UnitsNet/
MIT No Attribution
2.65k stars 382 forks source link

Introducing a [DebuggerTypeProxy] to the generated quantities #1390

Closed lipchev closed 5 months ago

lipchev commented 6 months ago
/// <summary>
///     Serves as a debug display proxy for a quantity, providing a convenient way to view various components of the
///     quantity during debugging.
/// </summary>
/// <remarks>
///     This struct provides a structured view of a quantity's components such as abbreviation, unit, value, and convertor
///     during debugging.
///     Each component is represented by a nested struct, which can be expanded in the debugger to inspect its properties.
/// </remarks>
internal readonly struct QuantityDisplay(IQuantity quantity)

This is added to all quantities such:

    [DataContract]
    [DebuggerTypeProxy(typeof(QuantityDisplay))]
    public readonly partial struct Mass :

There are 4 properties visible, each of which can be further expanded:

  1. Abbreviation: displays the DefaultAbbreviation and expands with the list all abbreviations for the current unit, also holds a property with the possible unit conversions (displaying the target abbreviation in the ConvertedQuantity[])
  2. Unit: displays the Unit such as Milligram and expands to the list of other options, where every other options expands to the respective ConvertedQuantity (much like a smart enum).
  3. Value: displays the DoubleValue by default and expands with the IsDecimal, DecimalValue and DoubleValue properties
  4. ValueConversions: displays the output of the QuantityToString by default and expands with the list of possible QuantityToString options (currently only has the GeneralFormat and ShortFormat properties) as well as the list of possible unit conversions (ConvertedQuantity[])

Note that I've had to carefully pick the property names such that they are sorted (more or less) how I wanted, as visual studio sorts them alphabetically, not by the order of declaration. Perhaps there (is / would one day be) an option to change that behavior but, yeah- that's it for now.

I've only tested this with Visual Studio 2022 - but i don't see why there should by any issues with the other IDEs. I did observe that not all things are always displayed on net48 (sometimes there is the _"..requires thread to complete..").

image image image image image

If we open the visualizer view with the list from the QuantityToUnit we get a nice little table:

image

lipchev commented 6 months ago

I initially had the Abbreviation put inside the Unit section but that ruined the smart enum effect. However now that I'm looking at it again, I think it might actually be better to have the Abbreviation named UnitAbbreviation (which would place it right underneath the Unit. It's a bit longer (as text) but the expansion using the keyboard (from the top) would be shorter.. I don't know- you decide..

angularsen commented 6 months ago

I tested in JetBrains Rider.

I like it a lot, the default view is very verbose with all conversion properties so this helps clean it up and you still have access the the raw debug output 👍

image

I like how you can "dig" through conversions and view the converted quantities 👍 image

I like how you can preview all the available conversions for the current selected quantity: image

I think maybe Abbreviation.Conversions feels redundant or not so useful? At least for me, digging through Units is more intuitive. image

Can we add QuantityInfo too, for completeness? Then I don't imagine ever having to expand the raw debug output for anything. We can skip Dimensions, since QuantityInfo already describes it.

ConvertedQuantity.Value could maybe reuse ValueDisplay to be consistent and avoid the decimal error? Or, can maybe ConvertedQuantity inherit from QuantityDisplay to get the same base output, just extending with whatever extra the converted quantity holds? image

lipchev commented 6 months ago

I think maybe Abbreviation.Conversions feels redundant or not so useful? At least for me, digging through Units is more intuitive.

For the quantities I'm familiar with, I tend to navigate using the Units as well, wasn't sure what's the more natural way for other/ new users. This is also why I've been thinking about changing the name to UnitAbbreviation so that I can get to my preferred unit conversion with just one click (from the top) of the right arrow key. However I do think there is some benefit in keeping the list separate from the version of the ValueConverter (perhaps under a different name, maybe without the conversion-drill-down). Have a look at this example: (visually) parsing the first half of the image takes a fraction from what it takes for the second half:

image

Here is the renamed version - it's just the Conversions property name that I find somewhat displeasing, but AbbreviationInOtherUnits didn't seem right either..

image

Can we add QuantityInfo too, for completeness? Then I don't imagine ever having to expand the raw debug output for anything. We can skip Dimensions, since QuantityInfo already describes it.

We can have anything, just need a name that would sort correctly in the display: the "Q" in QuantityInfo will have it appear between the Abbreviation and the Unit which is a bit annoying. I was also thinking of exposing the QuantityInfo with the {Name} or {PluralName} display by default, however I couldn't think of the name (or sub-section) to use for it..

ConvertedQuantity.Value could maybe reuse ValueDisplay to be consistent and avoid the decimal error? Or, can maybe ConvertedQuantity inherit from QuantityDisplay to get the same base output, just extending with whatever extra the converted quantity holds?

Yes, I must have missed that (they are both displaying the same way in v6).

angularsen commented 5 months ago

it's just the Conversions property name that I find somewhat displeasing

I think it's fine, we can always revisit.

the "Q" in QuantityInfo will have it appear between the Abbreviation and the Unit which is a bit annoying.

For me, that's entirely fine. I'm not so focused on the order, as much as having a fairly short list of properties that is easy to read and dig through.

lipchev commented 5 months ago

@angularsen I think this should be good (for now): I've added the missing ValueDisplay and renamed the Abbreviation to UnitAbbreviation having the Unit enum appear on top.

angularsen commented 5 months ago

Perfect, can you also port this to v6 branch?

angularsen commented 5 months ago

Released in https://github.com/angularsen/UnitsNet/releases/tag/UnitsNet%2F5.51.0

lipchev commented 5 months ago

@angularsen I ported the modifications to v6, and was just having a closer look at the screenshots you posted from Rider- that thing about the fully.qualified.type.name preceding the actual-value, that's pretty annoying to look at IMO. Is that something that's configurable? I didn't really like how long the namespaces was when looking at it as the "right-side-column" (the VS way)- maybe we could do something to shorten the namespace (that would probably require moving each proxy into its own file)?

angularsen commented 5 months ago

I honestly did not notice, but sure, it may shorten the name a bit if you move the types so they are not nested, but it seems Rider shows a kind of breadcrumb in this scenario for the path I have expanded.

As for configurable, I can disable "fully qualified", but it only removes the leading "UnitsNet" part. For me, I would not bother changing this anyway. I'm fine with it as-is.

image

image

angularsen commented 5 months ago

Do you want to make any changes before I merge #1395 ?

lipchev commented 5 months ago

Nah, we can do it later (perhaps even making the UnitsNet.Display namespace public or something).