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

proposal: encoding/xml: add RawXML token #26756

Open
SamWhited opened this issue Aug 1, 2018 · 12 comments
Open

proposal: encoding/xml: add RawXML token #26756

SamWhited opened this issue Aug 1, 2018 · 12 comments

Comments

@SamWhited
Copy link
Member

@SamWhited SamWhited commented Aug 1, 2018

I have recently been experimenting with writing an XML encoder that (given a struct or other similar type) emits xml.Token's instead of bytes. In doing so I discovered that some XML could be encoded using structs that cannot currently be encoded using the "Encoder".EncodeToken method and that I couldn't finish my project without occasionally switching over to the struct based API. There is currently no equivalent in the SAX-like API for the ,innerxml tag on struct fields. This is useful as it both lets us encode invalid XML (which is unfortunately quite common, and it may be desirable to let Go's encoder work with broken XML encoders and decoders in the wild), and lets us encode pre-escaped XML without accidentally double-escaping (eg. if we know we already have valid XML such as & we can prevent it from being turned into &).
This is similar in spirit to the known safe types in the html/template package (eg. HTML or JS).

Proposal: https://golang.org/design/26756-rawxml-token

@SamWhited SamWhited added this to the Proposal milestone Aug 1, 2018
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 1, 2018

Change https://golang.org/cl/127435 mentions this issue: encoding/xml: add RawXML token type and tests

@golang golang deleted a comment from gopherbot Aug 1, 2018
@mvdan
Copy link
Member

@mvdan mvdan commented Aug 2, 2018

@mvdan
Copy link
Member

@mvdan mvdan commented Aug 2, 2018

@SamWhited have you considered making this a proposal? I'd assume that is the best approach if you want to add new APIs to a stable std package.

@SamWhited
Copy link
Member Author

@SamWhited SamWhited commented Aug 2, 2018

@mvdan I'm never really sure what the criteria for needing a full proposal are. This seemed straight forward enough to not require one (there's only one way that I can see to do this which is to add a new token type), but I'm also happy to write more about it.

@mvdan
Copy link
Member

@mvdan mvdan commented Aug 2, 2018

In this specific case I'd make it a proposal so that the proposal review committee makes a decision on it in the next few weeks. Otherwise, this issue might not get a decision for a while, as NeedsDecision issues are quite common.

I just noticed that you already added this issue to the proposal milestone. I'll add a prefix to the title, which is the better way to make it a proposal :) @gopherbot then takes care of the milestone and label for us.

@mvdan mvdan changed the title encoding/xml: add RawXML token Proposal: encoding/xml: add RawXML token Aug 2, 2018
@gopherbot gopherbot added the Proposal label Aug 2, 2018
@SamWhited
Copy link
Member Author

@SamWhited SamWhited commented Aug 2, 2018

I just noticed that you already added this issue to the proposal milestone. I'll add a prefix to the title, which is the better way to make it a proposal :) @gopherbot then takes care of the milestone and label for us.

Ah thanks, the gardening and issue pages no longer mention the proposal prefix at all that I could find, so I assumed it was deprecated. I think I've been doing this wrong elsewhere too.

@SamWhited
Copy link
Member Author

@SamWhited SamWhited commented Aug 2, 2018

That being said, having a milestone, a label, and a prefix seems like overkill, now that I think about it.

@mvdan
Copy link
Member

@mvdan mvdan commented Aug 2, 2018

Maybe so :) send an email to golang-dev? Maybe the prefix is needed for a specific reason that I'm not aware of. Or perhaps, like you say, we should get rid of it.

The label does seem unnecessary given the milestone, though. I'm just following what pretty much all the other proposals do.

@SamWhited
Copy link
Member Author

@SamWhited SamWhited commented Aug 2, 2018

Yah, might be worth discussing in any case. If I get around to it later I'll send an email (and start writing up a more formal proposal for this). Thanks for the feedback!

@rsc rsc changed the title Proposal: encoding/xml: add RawXML token proposal: encoding/xml: add RawXML token Aug 6, 2018
@iWdGo
Copy link
Contributor

@iWdGo iWdGo commented Aug 22, 2018

Proposal is similar to #6825

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 1, 2018

Change https://golang.org/cl/132836 mentions this issue: design: add raw xml token proposal

gopherbot pushed a commit to golang/proposal that referenced this issue Sep 7, 2018
See golang/go#26756

Change-Id: I6a2b2fae94d71a784a2489e0af8ca2df4b57bc4c
Reviewed-on: https://go-review.googlesource.com/132836
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@rsc
Copy link
Contributor

@rsc rsc commented Oct 17, 2018

On hold for XML sweep.

@rsc rsc added the Proposal-Hold label Oct 17, 2018
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
5 participants
You can’t perform that action at this time.