Skip to content

Fix remarks for JsonElement TryGetDouble and TryGetSingle #3719

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

Merged
merged 4 commits into from
Dec 31, 2019

Conversation

ahsonkhan
Copy link
Contributor

@ahsonkhan ahsonkhan commented Dec 28, 2019

Add missing `"` and change `it returns false` to `it returns <see langword="true">`
@dotnet-bot dotnet-bot added this to the December 2019 milestone Dec 28, 2019
@ahsonkhan
Copy link
Contributor Author

Btw, what do I need to do to get write/merge privileges in this repo? cc @carlossanlop

@@ -1515,7 +1515,7 @@ This method does not parse the contents of a JSON string value.

This method does not parse the contents of a JSON string value.

On .NET Core, this method does not return `false` for values larger than <xref:System.Single.MaxValue?displayProperty=nameWithType> or smaller than <xref:System.Single.MinValue?displayProperty=nameWithType>). Instead, it returns `false` and assigns <xref:System.Single.PositiveInfinity?displayProperty=nameWithType> or <xref:System.Single.NegativeInfinity?displayProperty=nameWithType> to the `value` argument.
On .NET Core, this method does not return `false` for values larger than <xref:System.Single.MaxValue?displayProperty=nameWithType> or smaller than <xref:System.Single.MinValue?displayProperty=nameWithType>). Instead, it returns <see langword="true"> and assigns <xref:System.Single.PositiveInfinity?displayProperty=nameWithType> or <xref:System.Single.NegativeInfinity?displayProperty=nameWithType> to the `value` argument.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@carlossanlop - any ideas what caused the typo? Was these hand-written or does it point to a potential porting tool bug?

These were added in #2471.

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

It wasn't rendering properly. You can check the preview here.

ahsonkhan and others added 2 commits December 30, 2019 17:26
Co-Authored-By: Genevieve Warren <gewarren@microsoft.com>
Co-Authored-By: Genevieve Warren <gewarren@microsoft.com>
Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

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

The 'true' still wasn't rendering. It seems those <see langword="" /> tags aren't supported in the Remarks section.

Co-Authored-By: Genevieve Warren <gewarren@microsoft.com>
@ahsonkhan
Copy link
Contributor Author

The 'true' still wasn't rendering. It seems those <see langword="" /> tags aren't supported in the Remarks section.

Hmm, well that's unfortunate. Do you know why the remarks section is handled differently? We use langword in a bunch of places for exception/summary tags, so I would have thought remarks would behave the same way.

@gewarren
Copy link
Contributor

The 'true' still wasn't rendering. It seems those <see langword="" /> tags aren't supported in the Remarks section.

Hmm, well that's unfortunate. Do you know why the remarks section is handled differently? We use langword in a bunch of places for exception/summary tags, so I would have thought remarks would behave the same way.

I don't know why the Remarks section is different. @mairaw?

@gewarren gewarren merged commit 10a541b into dotnet:master Dec 31, 2019
@ahsonkhan ahsonkhan deleted the patch-1 branch January 1, 2020 01:24
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.

3 participants