File: reduce scope of whitespace trimming#919
Merged
Conversation
Merged
Alkarex
reviewed
Jun 26, 2025
Alkarex
reviewed
Jun 26, 2025
9d6c3c1 to
a812eb3
Compare
Member
Author
|
Also added a regression test. |
Member
Author
|
By the way, I do not think File is correct place for this, as it won’t work with |
Alkarex
approved these changes
Jun 27, 2025
Alkarex
reviewed
Jun 27, 2025
018b5ca to
fcb82db
Compare
Alkarex
reviewed
Jun 27, 2025
e2dc6d8 to
da824c0
Compare
da824c0 to
2e8011b
Compare
2e8011b to
ca01c0f
Compare
ca01c0f to
aa15f55
Compare
Alkarex
reviewed
Jun 29, 2025
1c14ebd to
ddd2a8d
Compare
jtojnar
added a commit
that referenced
this pull request
Jul 29, 2025
mf2 library uses implicit nullable types, which were deprecated in PHP 8.4: https://wiki.php.net/rfc/deprecate-implicitly-nullable-types This causes the response body of the HTTP mock server in tests to be polluted with the following warning: Deprecated: Mf2\Parser::parse(): Implicitly marking parameter $context as nullable is deprecated, the explicit nullable type must be used instead in vendor/mf2/mf2/Mf2/Parser.php on line 1373 This breaks tests in <#838> and <#919>. Until the upstream fix (microformats/php-mf2#264) is merged, let’s uninstall mf2 in PHP 8.4 CI. Works around: #921 Co-authored-by: Artur Weigandt <Art4@users.noreply.github.com>
ddd2a8d to
cec0005
Compare
Art4
approved these changes
Jul 29, 2025
Trimming final whitespace would mangle UTF-16BE encoded files ending with whitespace. For example a new-line (`\x00\x0A`), would be turned into half a code-point `\x00`. Let’s just trim at the beginning of the file. Similarly, trimming initial whitespace can be issue with UTF-16LE (`\x0A\x00` would again get mangled into `\x00`). Let’s ensure that the whitespace is immediately followed by `<`. The trim was introduced in 989645e since XML declaration cannot be preceded by anything other than BOM, and XML declaration starts with `<`, so the requirement should be fine. It might still mangle body of some weird-ass encoding but that is unlikely to be encountered nowadays. This is a follow-up to 458d746.
It was introduced in 989645e and further refined in the parent commit.
This is a regression test for 7206ab3.
cec0005 to
d38e63d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trimming final whitespace would mangle UTF-16BE encoded files ending with whitespace. For example a new-line (
\x00\x0A), would be turned into half a code-point\x00. Let’s just trim at the beginning of the file.Similarly, trimming initial whitespace can be issue with UTF-16LE (
\x0A\x00would again get mangled into\x00). Let’s ensure that the whitespace is immediately followed by<.The trim was introduced in #445 since XML declaration cannot be preceded by anything other than BOM, and XML declaration starts with
<, so the requirement should be fine.It might still mangle body of some weird-ass encoding but that is unlikely to be encountered nowadays.
Also add a regression test.
This is a follow-up to #917.
Depends on #918