feat: PixelSize TryParse

[workgroupengineering]

What does the pull request do?

Add TryParse method to PixelSize

What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Breaking changes

Obsoletions / Deprecations

Fixed issues

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046192-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[MrJul]

While I have nothing against the API itself, why do we have the ability to provide the separator in TryParse but not in Parse?

In my opinion, I don't think the separator should be able to be specified at all for these methods.
There should be only one supported format, allowing for [Try]Parse(ToString()) to round trip. (Others in the team might have different thoughts on the matter.)

We should also think about consistency here. Why don't we have TryParse for Point, Size, Rect etc? Of course I'm not asking you to add all these to this PR, that's out of scope! But we should really think about having consistent API everywhere, which makes for a better user experience.

[MrJul]

Also, shouldn't we rewrite Parse by using TryParse, avoiding logic duplication?

[workgroupengineering]

We should also think about consistency here. Why don't we have TryParse for Point, Size, Rect etc? Of course I'm not asking you to add all these to this PR, that's out of scope! But we should really think about having consistent API everywhere, which makes for a better user experience.

My intention is to gradually add the TryParse method to the other types.

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046201-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[MrJul]

I still don't really like the separator being a parameter. As mentioned before, we should only allow the format currently used in XAML. Also, the separator may be ignored since a whitespace will always act as an extra separator in the current StringTokenizer implementation. This might confuse users: "the separator I've specified isn't respected!".

For more complex types it may even clash with the decimal separator or other special characters from the format. Since we don't allow the culture to be specified (for good reasons, we're parsing a known format here, not accepting localized user input), the separator shouldn't be either.

Pinging @maxkatz6 for a second opinion on this matter.

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046251-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[hez2010]

Maybe you can also provide an overload for ReadOnlySpan<char>?

[maxkatz6]

ReadOnlySpan overload would be nice in general, but we don't have it anywhere else. Could be a candidate for the future PRs.

Pinging @maxkatz6 for a second opinion on this matter.

I would vote to remove separator parameter. As pixel size parsing is always invariant (with '.' decimal point), and ',' as a separator is well common. If it's ever needed, we can add another overload with separator in the future, but not necessary now.

[maxkatz6]

Also, CultureInfo.InvariantCulture.TextInfo.ListSeparator exists, if we would need to pass IFormatProvider to Parse method.
Not sure if it's the best application of ListSeparator, but WPF seems to use it for non-invariant cultures: https://github.com/dotnet/wpf/blob/85d908ce8599d12f5e16ce7d78083cfdc2e13af4/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/ThicknessConverter.cs#L167C50-L176

[kekekeks]

For ReadOnlySpan overloads we'll need to implement some shims for netstandard2.0 that would parse integers/doubles for invariant locale without actually allocating a string. Can probably copy-paste some code from BCL.

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046758-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[MrJul]

I would vote to remove separator parameter. As pixel size parsing is always invariant (with '.' decimal point), and ',' as a separator is well common. If it's ever needed, we can add another overload with separator in the future, but not necessary now.

@workgroupengineering Can you please remove the separator argument from this PR?

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046930-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046936-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046944-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-bot]

You can test this PR using the following package version. 11.2.999-cibuild0046971-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]