Page MenuHomePhabricator

Figure out if new empty line treatment really improves things
Closed, ResolvedPublic8 Estimated Story Points


NB this is timeboxed to 5 hours

Figure out if we really create an improvement in diffs by introducing the new empty line treatment. To do so, please follow the process we established for changes in moved paragraphs, which means:

  • generate random article samples
  • check if they were improved / worsened/ unchanged (imagining we had implemented the whole feature, where line numbers are correct and emptly lines are seperate visual entities)
  • document the results

Event Timeline

Lea_WMDE triaged this task as Medium priority.Feb 6 2018, 4:49 PM
Lea_WMDE created this task.
Lea_WMDE updated the task description. (Show Details)
Lea_WMDE added a subscriber: WMDE-Fisch.
Tobi_WMDE_SW set the point value for this task to 8.Feb 20 2018, 4:28 PM

During my investigation on that topic I got some regression on the following diffs. These can be used as examples to look into way to improve the issue.

Removed empty line and small text involved:

Screenshot-2018-2-21 „Brot“ – Versionsunterschied – Wikipedia.png (430×1 px, 44 KB)
Screenshot-2018-2-21 Difference between revisions of Brot - EnLocalWiki.png (487×1 px, 48 KB)

Move of a line above an empty line:

Screenshot-2018-2-21 „Brot“ – Versionsunterschied – Wikipedia(1).png (634×1 px, 58 KB)
Screenshot-2018-2-21 Difference between revisions of Brot - EnLocalWiki(1).png (827×1 px, 72 KB)

Adding an empty line and text:

Screenshot-2018-2-21 „Brot“ – Versionsunterschied – Wikipedia(2).png (703×1 px, 64 KB)
Screenshot-2018-2-21 Difference between revisions of Brot - EnLocalWiki(2).png (631×1 px, 88 KB)

WMDE-Fisch added subscribers: Andrew-WMDE, Addshore.

Since we decided it might be better, if someone else looks at a way to implement this. I unassign myself here @jkroll @Addshore @Andrew-WMDE. The examples above can be used as edge cases that need to be taken care off.

Lea_WMDE claimed this task.
Lea_WMDE moved this task from Sprint backlog to Done on the WMDE-QWERTY-Sprint-2018-02-20 board.

This has been resolved differently by looking at paragraphs that have only been moved one line. For more details see the patch posted below.

unfortunately, the diffs generated using this special-case code turned about to be incorrect so i had to revert this. looking at the list of random test diffs, these cases turn out to be rare though.