-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add MIME::MediaType #7077
Add MIME::MediaType #7077
Conversation
fd7b793
to
e01f1aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs code comments on the "tricky bits", especially the rfc2231 handling
src/mime/media_type.cr
Outdated
@@ -5,8 +5,7 @@ require "mime" | |||
module MIME | |||
# A MediaType describes a MIME content type with optional parameters. | |||
struct MediaType | |||
property media_type : String | |||
property params : Hash(String, String) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Having access to raw params Hash could come handy, when removed there's no way to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MediaType
directly exposes #[]
, #[]?
, #fetch
, #[]=
and #each_parameter
. Why would yo need direct access to params
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For serialization as an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What kind of serialization? I figure you'd either serialize with #to_s
or directly access the ivar @params
.
ffb4b52
to
da6ce50
Compare
90de96c
to
92d2b18
Compare
@RX14 I removed |
92d2b18
to
3663040
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still like many more code comments to explain the implicit state while parsing. Especially the rfc2231 stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally, thank you @straight-shoota 👍
Squash please! |
c083d5f
to
623ffca
Compare
623ffca
to
f4f1151
Compare
This PR adds a
MIME::MediaType
type with methods for parsing mime media types as per RFC 2045 and RFC 2046 including support for optional parameters.This type is put to use for reading and writing HTTP
Content-Type
headers andHTTP::Multipart
boundaries replacing existing incomplete implementations.This is not directly integrated with the
MIME
registry (added in #5765) but can be optionally used together.Closes #5686