Fix bug in adding references in CSV format
ClosedPublic

Authored by Lucas_Werkmeister_WMDE on May 6 2018, 8:50 AM.

Details

Reviewers
Magnus
Patch without arc
git checkout -b D1050 && curl -L https://phabricator.wikimedia.org/D1050?download=true | git apply
Summary

$lastSources is a reference (in the PHP sense – &value) so that adding further snaks to the previous source (with s123) works, which means that any time we assign to it, we must first break the reference using unset() (compare L1058 and L1103). This was missing in the beginning of the loop body, with the consequence that in a multi-line import, the last reference of each line except the last would be broken (it would have been reset to null by the next line).


Here’s what the bug looks like in action (note the missing sources for the date of death):


The commit is also available on GitHub: https://github.com/lucaswerkmeister/quickstatements/commit/d31e272e5bc3607584a87d7bc5d357ed8472c015

Improved “patch without arc”:

git checkout -b D1050 &&
curl -L https://github.com/lucaswerkmeister/quickstatements/commit/d31e272e5bc3607584a87d7bc5d357ed8472c015.patch | git am

Diff Detail

Repository
R2010 tool-quickstatements
Lint
Lint Skipped
Unit
Unit Tests Skipped
Lucas_Werkmeister_WMDE requested review of this revision.May 6 2018, 8:50 AM
Lucas_Werkmeister_WMDE created this revision.
Lucas_Werkmeister_WMDE edited the summary of this revision. (Show Details)May 6 2018, 8:55 AM
Lucas_Werkmeister_WMDE edited the summary of this revision. (Show Details)
Lucas_Werkmeister_WMDE edited the summary of this revision. (Show Details)
Lucas_Werkmeister_WMDE edited the summary of this revision. (Show Details)May 6 2018, 9:01 AM
Magnus accepted this revision.May 7 2018, 6:25 PM
This revision is now accepted and ready to land.May 7 2018, 6:25 PM

Applied manually by Magnus. (I still don’t really understand Differential, but this revision still seems to be open, which I don’t think it should be anymore.)