Some of the possible action items I can see us working on during the WMDE-QWERTY-X-Mas-Sprint-2018-12-18:

• Pretty much all new Message() in the code should be replaced with $this->msg(). The problem with new Message() is that it relies on global state (the $wgUsers settings as well as the wikis configuration). It is expensive and hard to properly test because of this.
• While the test coverage is superb (>80%, see https://doc.wikimedia.org/cover-extensions/TwoColConflict/), TwoColConflictHooks is uncovered. Instead of coming up with an arbitrary unit test, we might think of this code as being covered by the browser tests, and mark the methods we think are sufficiently covered via @codeCoverageIgnore.
• Why is LineBasedUnifiedDiffFormatter a subclass of DiffFormatter? As far as I can see it does not need to. Removing the dependency would free us from certain constraints and unpredictable side-effects.
• The big switch-case in LineBasedUnifiedDiffFormatter::format needs some love to become easier to read. Note how two cases call private $this->addLines/deleteLines, but one does not. An approach I personally like to follow is to inline such trivial methods first, which allows me to play around with entirely new ways to restructure and possibly split the code.
• LineBasedUnifiedDiffFormatter::format gets a \Diff objects and converts it to an intermediate array format, which is then consumed by HtmlSplitConflictView::getHtml, which also has a big, hard to read switch-case. The intermediate format contains a lot of duplicate information, some already in HTML form. It might be worth rethinking this intermediate format, reducing the amount of duplication, and striving for a more clear separation between:
  • … code that knows what a \Diff object is, and how it is structured.
  • … code that knows what we want to show on our conflict resolution screen, and the intermediate representation we use for it.
  • … code that creates HTML.
• …

Please extend as needed.