Skip to content

ignore case when looking for "charset"#4723

Merged
andralex merged 3 commits intodlang:masterfrom
aG0aep6G:curl-response-charset
Aug 10, 2016
Merged

ignore case when looking for "charset"#4723
andralex merged 3 commits intodlang:masterfrom
aG0aep6G:curl-response-charset

Conversation

@aG0aep6G
Copy link
Contributor

@aG0aep6G aG0aep6G commented Aug 8, 2016

RF2616 says: "The type, subtype, and parameter attribute names are case-insensitive."

https://tools.ietf.org/html/rfc2616#section-3.7

See also: http://forum.dlang.org/post/scoeormeciaoiqxsefkz@forum.dlang.org.

@wilzbach
Copy link
Contributor

wilzbach commented Aug 9, 2016

@aG0aep6G I just saw your style fixes ;-)

fix selective import style

FWIW at #4719 there's a make target (style).

@codecov-io
Copy link

Current coverage is 88.74% (diff: 100%)

Merging #4723 into master will decrease coverage by <.01%

@@             master      #4723   diff @@
==========================================
  Files           121        121          
  Lines         74037      74037          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          65705      65704     -1   
- Misses         8332       8333     +1   
  Partials          0          0          

Powered by Codecov. Last update 4a634ed...f87d112

format("HTTP request returned status code %d (%s)",
statusLine.code, statusLine.reason));

// Default charset defined in HTTP RFC
Copy link
Member

@DmitryOlshansky DmitryOlshansky Aug 9, 2016

Choose a reason for hiding this comment

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

Love the red but ... was it basically useless work duplicated elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Love the red but ... was it basically useless work duplicated elsewhere?

The charset detection is already done in the other location that's touched by this PR: HTTP.onReceiveHeader. The function here calls that.

@DmitryOlshansky
Copy link
Member

LGTM

@andralex
Copy link
Member

nicely done

@andralex
Copy link
Member

Auto-merge toggled on

@andralex andralex merged commit b2aacbf into dlang:master Aug 10, 2016
@aG0aep6G aG0aep6G deleted the curl-response-charset branch August 11, 2016 21:30
@wilzbach
Copy link
Contributor

@aG0aep6G ideally we can link every bug fix with a bugzilla issue, s.t. the end users sees what amazing work you / we did :)

@aG0aep6G
Copy link
Contributor Author

aG0aep6G commented Aug 31, 2016

@aG0aep6G ideally we can link every bug fix with a bugzilla issue, s.t. the end users sees what amazing work you / we did :)

You mean so that the users see what embarrassing bugs we used to have ;)

Can I retroactively link this to a bugzilla issue? Does the changelog script pick that up? I don't think a simple bug fix like this needs an entry in the prose section.

@wilzbach
Copy link
Contributor

You mean so that the users see what embarrassing bugs we used to have ;)

Hehe no that they see what progress we make ;-)

Can I retroactively link this to a bugzilla issue? Does the changelog script pick that up? I don't think a simple bug fix like this needs an entry in the prose section.

AFAIK it's possible. I just created an issue & will check that the changelog script picks it up.

@aG0aep6G
Copy link
Contributor Author

AFAIK it's possible. I just created an issue & will check that the changelog script picks it up.

Ok, thanks. I'll try to be more diligent in also filing issues when doing fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants