Problem/Motivation
The \Drupal\Component\Diff\DiffOpOutputBuilder implementation added in #3299678: Deprecate DiffEngine and replace with sebastian/diff doesn't match the ouput of 10.0.x diffing. This results in test failures for the update_helper module on 10.1.x.
Steps to reproduce
$a = <<<EOF
settings
settings::max_length : i:123;
status : b:0;
translatable : b:1;
EOF;
$b = <<<EOF
settings : a:0:{}
status : b:1;
translatable : b:1;
EOF;
$a = explode("\n", $a);
$b = explode("\n", $b);
$diff = new \Drupal\Component\Diff\Diff($a, $b);
dump($diff->getEdits());
On 10.0.x this produces:
^ array:2 [
0 => Drupal\Component\Diff\Engine\DiffOpChange^ {#2855
+type: "change"
+orig: array:3 [
0 => "settings"
1 => "settings::max_length : i:123;"
2 => "status : b:0;"
]
+closing: array:2 [
0 => "settings : a:0:{}"
1 => "status : b:1;"
]
}
1 => Drupal\Component\Diff\Engine\DiffOpCopy^ {#2915
+type: "copy"
+orig: array:1 [
0 => "translatable : b:1;"
]
+closing: array:1 [
0 => "translatable : b:1;"
]
}
]
On 10.1.x this produces:
^ array:3 [
0 => Drupal\Component\Diff\Engine\DiffOpChange^ {#1898
+type: "change"
+orig: array:2 [
0 => "settings"
1 => "settings::max_length : i:123;"
]
+closing: array:2 [
0 => "settings : a:0:{}"
1 => "status : b:1;"
]
}
1 => Drupal\Component\Diff\Engine\DiffOpDelete^ {#1930
+type: "delete"
+orig: array:1 [
0 => "status : b:0;"
]
+closing: false
}
2 => Drupal\Component\Diff\Engine\DiffOpCopy^ {#1727
+type: "copy"
+orig: array:1 [
0 => "translatable : b:1;"
]
+closing: array:1 [
0 => "translatable : b:1;"
]
}
]
Proposed resolution
This is due to how DiffOpOutputBuilder changes a series of deletions and additions into a change.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | 3386482-2.test-only.patch | 676 bytes | alexpott |
| #3 | 3386482-2.no-coverage.patch | 928 bytes | alexpott |
Issue fork drupal-3386482
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
alexpottThere's a bug in \Drupal\Component\Diff\DiffOpOutputBuilder::toOpsArray when it determines what diff ops to add. Patch attached is just the minimum fix to show there is no test coverage. And patch added to show that the added test coverage works.
Comment #4
alexpottComment #5
alexpottGiven this bug results in unexpected changes to diff produced by contrib modules I think that this is at least a major until proven otherwise.
Comment #7
alexpottThe test only patch failed as expected and the other patch and MR passed as expected.
Comment #8
mondrakeRTBC the MR, the NW for test failure is due to failing test only patch.
A missing test case was added, and code changes do not impact the existing test cases. Therefore good to go for me.
Comment #9
mondrake#8 xposted with #7
Comment #12
catchCommitted/pushed to 11.x and cherry-picked to 10.1.x, thanks!