Tgr added a comment.

DifferenceEngine is a god class which does everything from interpreting query parameters for selecting the target revision to direct DB queries to table formatting to writing to OutputPage. Also extensions can replace it with a subclass. So a proper solution will involve a full rewrite which splits DifferenceEngine into a controller that determines which slots to diff, the actual diff generation code, and a layout generator which does headings and whatnot. Also probably some kind of composition logic that might vary the same way the composition logic for content rendering does. And BC shims since at least the diff generation code (but probably also some of the layout) lives in the extensions. That's probably way too much for phase 1.

A hacky solution that just makes a few DifferenceEngine methods iterate through multiple contents does not seem too hard; some questions that came up in the process:

  • The DifferenceEngine is selected by the ContentHandler, even though the old and new content is not guaranteed to be the same content type. (As a quirk of that, oldid=1&diff=next and oldid=2&diff=prev might in theory end up being completely different renderings.) There are multiple ways of handling that with slots:
    • Make diff engine selection the responsibility of the slot handler instead. IMO the sanest approach; could get messy if multiple extensions share a slot but that seems like an unusual thing to do.
    • Force non-main slots to always have the same content model. There are use cases that would be prevented by that (e.g. wikitext vs. Markdown for the documentation slot).
    • Keep the current behavior (maybe make it less chaotic by always using the old revision to select the engine).
  • Currently when diffing the first revision to its predecessor, no diff is shown, just an error message. When looking at a diff adding a new slot, that seems unhelpful. So we might need new DifferenceEngine functionality for generating a "null diff".
  • Currently the DifferenceEngine can personalize the diff header (tools like undo or link to previous). How will that look when there is a separate engine for each changed slot?

TASK DETAIL
https://phabricator.wikimedia.org/T174036

EMAIL PREFERENCES
https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Tgr
Cc: Liuxinyu970226, TomT0m, Smalyshev, Lokal_Profil, -jem-, Aklapper, daniel, Lahi, PDrouin-WMF, Gq86, E1presidente, Ramsey-WMF, Cparle, SandraF_WMF, GoranSMilovanovic, QZanden, Tramullas, Acer, LawExplorer, JJMC89, Agabi10, Susannaanas, Aschroet, Fjalapeno, Jane023, Wikidata-bugs, PKM, Base, matthiasmullie, aude, Ricordisamoa, Lydia_Pintscher, Fabrice_Florin, Raymond, Anomie, Steinsplitter, Mbch331
_______________________________________________
Wikidata-bugs mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

Reply via email to