Fix Aspell output parsing when the original words contains a colon #25
Conversation
| $issues [] = $issue; | ||
| $matches = []; | ||
| $pattern = '/^& (?<original>[^\s]+) \d+ (?<offset>\d+): (?<suggestions>.*)$/'; | ||
| if (1 === preg_match($pattern, $line, $matches)) { |
There was a problem hiding this comment.
Not sure what to do if it doesn't match the pattern 🤔
There was a problem hiding this comment.
If it's not matching, previously we wouldn't have anything in $parts. So that would be an empty array.
I would say the line is simply ignored.
But you could write a test for that where the Ispell output doesn't match. And see if it breaks :)
| $issues [] = $issue; | ||
| $matches = []; | ||
| $pattern = '/^& (?<original>[^\s]+) \d+ (?<offset>\d+): (?<suggestions>.*)$/'; | ||
| if (1 === preg_match($pattern, $line, $matches)) { |
There was a problem hiding this comment.
If it's not matching, previously we wouldn't have anything in $parts. So that would be an empty array.
I would say the line is simply ignored.
But you could write a test for that where the Ispell output doesn't match. And see if it breaks :)
tests/Unit/Aspell/AspellTest.php
Outdated
| public function testCheckTestWithColon(): void | ||
| { | ||
| $source = $this->prophesize(EncodingAwareSource::class); | ||
| $source->getEncoding()->shouldBeCalled()->willReturn('UTF-8'); |
There was a problem hiding this comment.
You don't need to add shouldBeCalled() to every method your are mocking.
It is not necessary here because the only thing you want to provide is the string value.
What you could have also done is using a instance of StringSource
E.g.
$source = new StringSource('S:t Petersburg är i Ryssland');Because this is simply a "DataObject" which doesn't need mocking.
|
Nice. Thanks for the fix 👍 |
|
Tagged with 2.1.1. Thanks again 👏 |
Fixes #24