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: net/texproto: add missing MIMEHeader.Write() #16479

Closed
emersion opened this issue Jul 23, 2016 · 10 comments
Closed

proposal: net/texproto: add missing MIMEHeader.Write() #16479

emersion opened this issue Jul 23, 2016 · 10 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal
Milestone

Comments

@emersion
Copy link

http.Header is using many methods from textproto.MIMEHeader (see https://golang.org/src/net/http/header.go), but textproto.MIMEHeader is missing a Write method. This method is implemented in net/http. It seems weird to import net/http each time a textproto.MIMEHeader needs to be formatted. The Write method could instead be moved to net/textproto, and called in net/http as it's done for several other functions.

@quentinmit quentinmit changed the title Move net/http.Header.Write() to net/textproto.MIMEHeader.Write() net/http, net/textproto: Move net/http.Header.Write() to net/textproto.MIMEHeader.Write() Jul 29, 2016
@quentinmit quentinmit added this to the Go1.8Maybe milestone Jul 29, 2016
@quentinmit
Copy link
Contributor

For backwards-compatibility reasons we'll have to keep around a Header.Write, of course.

@gopherbot
Copy link

CL https://golang.org/cl/25341 mentions this issue.

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@bradfitz
Copy link
Contributor

Copy of comment from CL:

Do we really want to add a WriteSubset method to the textproto package? It's weird enough in net/http in retrospect. If textproto needs this functionality, then I'd rather move the bulk of the logic to an internal shared package so we don't need to expose WriteSubset.

@emersion
Copy link
Author

So the idea would be to add WriteSubset to net/internal?

Another solution would be to implement Write without WriteSubset in textproto, and keep the WriteSubset implementation in http.

@bradfitz
Copy link
Contributor

As long as we don't add more public API.

@rsc
Copy link
Contributor

rsc commented Oct 20, 2016

Let's leave things alone for Go 1.8.

@rsc rsc modified the milestones: Go1.9, Go1.8Maybe Oct 20, 2016
@bradfitz bradfitz modified the milestones: Go1.10, Go1.9 Jun 5, 2017
@bradfitz bradfitz modified the milestones: Go1.10, Unplanned Nov 15, 2017
@odeke-em
Copy link
Member

odeke-em commented Sep 2, 2020

Looks like the CL https://go-review.googlesource.com/c/go/+/25341/ was abandoned with a reasoning of we don't want to make the change, cc @rsc

@emersion
Copy link
Author

emersion commented Sep 2, 2020

The CL doesn't mention any NACK. Brad said we shouldn't expose WriteSubset, but that doesn't mean the CL is rejected.

@odeke-em
Copy link
Member

@emersion, @rsc replied with

The author has not replied since PS1 when I asked him/her to update the Author and Committer lines, and also we don't believe we want to do this. So -2 and abandoning.

That was my direct reading of the abandonment.

Also adding that method without a proposal is no longer feasible, so I am going to repurpose this issue as a proposal that’ll go through a review.

@odeke-em odeke-em changed the title net/http, net/textproto: Move net/http.Header.Write() to net/textproto.MIMEHeader.Write() proposal: net/texproto: add missing MIMEHeader.Write() Mar 10, 2021
@emersion
Copy link
Author

I have no longer interest in this.

@golang golang locked and limited conversation to collaborators Mar 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal
Projects
None yet
Development

No branches or pull requests

6 participants