Fix GH-19188: Add support for new INI mail.cr_lf_mode#19238
Fix GH-19188: Add support for new INI mail.cr_lf_mode#19238alexandre-daubois merged 1 commit intophp:masterfrom
mail.cr_lf_mode#19238Conversation
bukka
left a comment
bukka
left a comment
There was a problem hiding this comment.
It looks good. Just few minor things.
alexandre-daubois
force-pushed
the
gh-19188
branch
2 times, most recently
from
20f3b35 to
4323731
Compare
alexandre-daubois
force-pushed
the
gh-19188
branch
from
4323731 to
a50a3ff
Compare
bukka
left a comment
bukka
left a comment
There was a problem hiding this comment.
I went through it again few times and don't see any issue. All good though.
@php/release-managers-85 are you ok with this going to 8.5? It should hopefully resolve issues for some mail users.
DanielEScherzer
left a comment
DanielEScherzer
left a comment
There was a problem hiding this comment.
Adding a new INI option seems like something that should have an internals discussion, if not a full RFC
But I'll leave the call to @edorian about if this is small enough to go into 8.5
edorian
left a comment
edorian
left a comment
There was a problem hiding this comment.
Doesn't seem controversial, PR is open for a long time. Given that, I don't see much use on delaying this for a year.
So 👍 from me for 8.5
alexandre-daubois
force-pushed
the
gh-19188
branch
from
a50a3ff to
17d3212
Compare
| if (ZSTR_LEN(new_value) > 0 && | ||
| strcmp(val, "crlf") != 0 && | ||
| strcmp(val, "lf") != 0 && | ||
| strcmp(val, "mixed") != 0 && | ||
| strcmp(val, "os") != 0) { |
There was a problem hiding this comment.
This could've been zend_string_equals_literal() to use the higher-level zend_string APIs (making the code more readable).
5 participants
Fix #19188