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

[12.x] Make Str::is() match multiline strings #51196

Merged
merged 4 commits into from
Apr 25, 2024
Merged

[12.x] Make Str::is() match multiline strings #51196

merged 4 commits into from
Apr 25, 2024

Conversation

SjorsO
Copy link
Contributor

@SjorsO SjorsO commented Apr 24, 2024

Currently Str::is() almost always returns false for strings containing a newline, for example:

$multilineString = <<<'STRING'
first
second
third
STRING;

Str::is('*', $multilineString); // currently false
Str::is('first*', $multilineString); // currently false
Str::is("first\n*", $multilineString); // currently false
Str::is("first\nsecond\n*", $multilineString); // currently true but not very practical

This happens because the * wildcard gets changed to the regex pattern .*, but the . doesn't match newlines because the s modifier isn't present.

Str::is() is used by the framework to match fake outputs for the Process facade. Currently you can't properly match a multiline command (as reported in #50158 and fixed with a hack in #50164). This PR fixes this problem properly. For example:

$result = Process::run(<<<'COMMAND'
git clone --depth 1 \
      --single-branch \
      --branch main \
      git://some-url .
'COMMAND');

Process::preventStrayProcesses();

// Currently this pattern will never match and your test will always fail.
// After this PR the pattern works as you'd expect.
Process::fake(['git clone*' => 'faked output']);

After this PR the * pattern always matches everything, so I've added the $pattern === '*' check because it is a minor speed boost.

I've targeted 12.x because this PR is a breaking change. If you rely on Str::is() or str()->is() returning false on multiline strings then you'll have to update your code.

@taylorotwell taylorotwell merged commit baed746 into laravel:master Apr 25, 2024
28 checks passed
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.

2 participants