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

encoding/json: clarify Decoder.InputOffset semantics #42571

Open
dsnet opened this issue Nov 13, 2020 · 1 comment
Open

encoding/json: clarify Decoder.InputOffset semantics #42571

dsnet opened this issue Nov 13, 2020 · 1 comment

Comments

@dsnet
Copy link
Member

@dsnet dsnet commented Nov 13, 2020

The documentation on Decoder.InputOffset currently says:

The offset gives the location of the end of the most recently returned token and the beginning of the next token.

This explanation is somewhat incorrect since the offset cannot be a single value if there are intervening whitespace in-between two JSON tokens. Furthermore, the json package implicitly handles colons and commas, which are technically JSON tokens according to RFC 7159, section 2. May the offset ever be past these implicit colons and commas, or will it always be before them? The "token" in this context is a somewhat ambiguous.

The documentation should be clarified to say one of the following:

  • The offset may be between the end of the most recently returned token (exclusive), and before the beginning of the next token that may be returned (inclusive).
  • The offset points to the next character immediately after the most recently returned token.
  • The offset points to the first character of the next token that may be returned.

I chose the phrase "may be returned" to indicate that colons and commas aren't included since they aren't ever returned by the json API. I haven't checked the implementation yet to see what it actually returns.

The first definition gives more flexibility to how the Decoder may actually be implemented. The latter two definitions gives the user more assurance about what InputOffset actually means.

Updates #29688

@dsnet dsnet added the Documentation label Nov 13, 2020
@mvdan
Copy link
Member

@mvdan mvdan commented Nov 13, 2020

I agree that the current definition is a bit ambiguous, happy to review a CL. I assume we should just document the current behavior.

@cagedmantis cagedmantis added this to the Backlog milestone Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.