-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
fix: add support for text/plain; charset=utf-8
#269
Conversation
494c28e
to
b58e3e7
Compare
text/plain; charset=utf-8
instead of text/plain
while maintaining backwards compatabilitytext/plain; charset=utf-8
text/plain; charset=utf-8
text/plain; charset=utf-8
text/plain; charset=utf-8
text/plain; charset=utf-8
We might have to find a better way for dealing with content-formats which contain additional parameters in the future. At the moment, only the part before the first |
@awwright: Would you still be interested in this issue? |
I recall the reason I didn't was because it could break backwards compatibility in applications that use merely |
Hmm, yeah, I am not so sure either. Maybe a more complex process (i. e. actually parsing the parameters) is needed to distinguish different content-formats, at least when they are given as an input. In any case, considering the potential breaking change it might be better to defer this PR to version 1.0.0. |
Pull Request Test Coverage Report for Build 1282106479
💛 - Coveralls |
b58e3e7
to
d8b9a14
Compare
d8b9a14
to
c1bd7c5
Compare
I now opened and merged #282 which deals (partly) with the parameters issue. I am closing this PR for now but Content-Format 0 should probably be revisited in the future. |
This PR fixes #33. It adds support for the Content-Format
text/plain; charset=utf-8
as an input parameter. The default output Content-Format will now be changed totext/plain; charset=utf-8
so this might actually be a breaking change. I hope, however, that other implementations will be able to handle this.