Problem/Motivation
Currently genericRevisionController lays the diff out as a table.
We want to have a plugin system that allows different layouts / ways to present the diff.
They can be enabled or disabled and reordered.
The button shows the enabled plugins, with the default one initially.
These plugins would make sense to me:
- Side by side diff, colored, with bold labels, like #4, but reduced vertical spacing
- Github style diff, with line numbers, single column, with bold labels
- HTML DOM tree based diff, newlines are ignored, bold labels + parent tag tree, tag children are indented
- Markdown, with bold labels, single or two col?
- Visual rendered document, no labels, single col
Formatting semantics could also be picked from per-content-instance field text format settings (markdown, HTML, ...) so they wouldn't need to be any specific layout selections...
Other modules can easily provide a plugin with default config.
On the other way, this approach simplifies things as individual plugins don't need to be artificially abstracted.
Proposed resolution
Introduce the DiffLayout Plugin System
Remaining tasks
#2789411: Remove default css to set github as default theme
#2789437: Move filter dropdown above the table
#2788153: Convert default theme to a LayoutPlugin
#2790841: Let the user sort layout plugins
#2790837: Introduce gitHub Layout plugin
#2795267: Allow revision navigation using the same layout plugin
#2795273: Refactor module css libraries
#2797279: Check for field access when diffing
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | Issue changes screenshot.png | 36.84 KB | jonathanshaw |
Comments
Comment #2
miro_dietikerNot sure how much pluggability / extensibility we should offer.
We should allow that a new plugin can reimplement everything completely.
The current existing contrib was able to extend the UI. If this is easily doable, we don't need a custom plugin.
Not sure what the "visual diff" will be asking for. That's why i added it as high prio to investigate.
And most importantly maintain it as part of the diff codebase. We need more examples / variants to maintain our architecture so it fits the real varying cases.
Comment #3
miro_dietikerSo it's possible this issue is highly related to the references...
Comment #4
jonathanshawFor my use case I will need to create a function that consumes the output of compareRevisions() and lays it out in a condensed diff like that used on drupal.org in issue comments to show the changes to the issue fields.
I'd be happy to contribute it to diff if you wanted more examples to test the architecture. I'd also be happy to modify the current table layout code so that these 2 layouts are swappable.
But I'd need some high-level direction about the basic architecture to use for this - whether these should be services or plugins or whatever.
Comment #5
miro_dietikerCan we have some screenshots about what different things we try to achieve?
Comment #6
jonathanshawThe current display of the "Node changes" module as seen on drupal.org seems like a good start for what I want to acheive.
If we had #2769513: Offer full visual word diff I'd like to have a cutoff for the Issue summary changes: if more than x lines changed, show a link to the full diff (the current behavior). If less than x lines changed, then show the diff of those changed lines.
Comment #7
miro_dietikerOK..
In TMGMT we also display changes inline when a source changes. We display a button if we detect source change and display the diff right below the content area to resolve the 3 way merge... But we used core components and didn't find something that fits from Diff..
What you propose is more a "teaser diff" - a new concept to what we do now.
I guess we need new abstraction and settings for this.
So for now we should focus on the diff detail page with its varying appearance (markdown, field diff, visual rendered, ...) in this issue.
That means, a separate page that renders a diff between two versions.
The overview is supposed to be extended by the handler issue. Handlers will be able to generate a first teaser for a revision.
Please open another separate issue about the teaser diff idea.
Comment #8
jonathanshawA more advanced proposal has now emerged in #2784381-10: Define how to display field labels on comparison.
Comment #9
miro_dietikerHere we go, based on a discussion in #2784381: Define how to display field labels on comparison
We want to have plugins (that can be ordered and disabled) that prepares effective diff for display.
Each plugin can then decide what builder it uses.
The builder itself won't need too much discussion. They will be kept simple and many aspects considered more internal.
I proposed some different ways how diffs can make sense.
For the release, we want to have the plugin architecture and the current examples.
Later we can then iterate on the plugins, improve their display, and offer new plugins.
Thus updating the issue summary.
Comment #10
johnchqueOK, then I think I gonna try this.
Comment #11
johnchqueChanged to be META.
Comment #12
johnchqueAdded first step.
Comment #13
johnchqueAdded some other issues that need to be done first so we can update the tests earlier.
Comment #14
johnchqueAdded two new steps that seems to be nice. :)
Comment #15
miro_dietikerAh OK, yeah now i get it...
sorting and enable / disable could be done in a follow-up.
and even populating the button could be a follow-up if things still just work.
Comment #16
johnchqueOne more issue. As soon as this one goes in there will be just one layout plugin left. :)
Comment #17
johnchqueTwo more issues that IMHO are important too.
Comment #18
johnchqueLast issue to complete the plugin system provided by diff.
Comment #19
miro_dietikerWhen sorting is done, we should close this meta issue.
The two other tasks are just additional plugins.
We should move them into the roadmap issue if not yet present.
Comment #20
johnchqueThen issues removed. I think we also need to check access.
Comment #21
johnchqueFinally done, small followups but the implementation works. :)
Comment #23
markusd1984 commentedAny chances to see a D7 port, please? This is a great enhancement of Diff!
For node revision comparison to visually see the changes in the content is a much better and easier user review experience, would be awesome to see this still become available for D7.