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

Should Utf8JsonReader expose whether a JSON string needs to be unescaped or not? #28421

Closed
ahsonkhan opened this issue Jan 14, 2019 · 5 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json backlog-cleanup-candidate An inactive issue that has been marked for automated closure.
Milestone

Comments

@ahsonkhan
Copy link
Member

The Utf8JsonReader already knows whether a particular JSON string needs to be unescaped. If it stored that fact (and publicly exposed it), users like JsonDocument would not need to duplicate that check (which means we save the search for a backslash in the json payload).

https://github.com/dotnet/corefx/blob/77096755443730962810662501f82550fdb4656d/src/System.Text.Json/src/System/Text/Json/Utf8JsonReader.cs#L760-L771

cc @bartonjs

@ahsonkhan
Copy link
Member Author

ahsonkhan commented Jan 16, 2019

Adding an internal field for JsonDocument to leverage does not require a public API change and should be done as an implementation detail similar to us storing 'e' when processing numbers: dotnet/corefx#34386. For that component (at least), marking as up-for-grabs.

@ahsonkhan
Copy link
Member Author

Adding an internal field for JsonDocument to leverage does not require a public API change and should be done as an implementation detail similar to us storing 'e' when processing numbers: dotnet/corefx#34386. For that component (at least), marking as up-for-grabs.

This was completed as part of dotnet/corefx#34485

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the Future milestone Feb 1, 2020
@ghost
Copy link

ghost commented Oct 15, 2021

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of the experimental issue cleanup initiative we are currently trialing in a limited number of areas. Please share any feedback you might have in the linked issue.

@ghost
Copy link

ghost commented Nov 5, 2021

This issue will now be closed since it had been marked no recent activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@ghost ghost closed this as completed Nov 5, 2021
@ahsonkhan
Copy link
Member Author

There is a related issue, which acts as a follow-up to this one: #45167

@ghost ghost removed the no-recent-activity label Nov 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2021
@eiriktsarpalis eiriktsarpalis added the backlog-cleanup-candidate An inactive issue that has been marked for automated closure. label Feb 18, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json backlog-cleanup-candidate An inactive issue that has been marked for automated closure.
Projects
None yet
Development

No branches or pull requests

3 participants