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

Add content type library #124

Merged
merged 5 commits into from
Oct 20, 2017
Merged

Conversation

donny-dont
Copy link

Consolidate content-type related code into a single library and start using it within Message.

///
/// Returns [fallback] if [charset] is null or if no [Encoding] was found that
/// corresponds to [charset].
Encoding encodingForCharset(String charset, [Encoding fallback = LATIN1]) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have ??, it's probably simpler to just return null if the encoding isn't found rather than having fallback parameters.

}

/// Modifies the media [type]'s [encoding].
MediaType modifyEncoding(MediaType type, Encoding encoding) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be used anywhere, and I'm not sure it would be worth factoring out even if it were...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to use it when creating the content-type header from a body and also its in the multipart stuff.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just doesn't seem like it provides substantial value over just inlining the definition.

return _contentLengthCache;
var contentLengthHeader = getHeader(headers, 'content-length');
if (contentLengthHeader == null) return null;
return _contentLengthCache = int.parse(contentLengthHeader);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally avoid using the return value of assignment expressions.

@nex3 nex3 merged commit 7b0eb3b into dart-lang:master Oct 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants