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

CommentFileSizeAuthor
#6 Issue changes screenshot.png36.84 KBjonathanshaw

Comments

jonathanjfshaw created an issue. See original summary.

miro_dietiker’s picture

Not 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.

miro_dietiker’s picture

So it's possible this issue is highly related to the references...

jonathanshaw’s picture

For 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.

miro_dietiker’s picture

Can we have some screenshots about what different things we try to achieve?

jonathanshaw’s picture

StatusFileSize
new36.84 KB

The 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.

miro_dietiker’s picture

OK..

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.

jonathanshaw’s picture

miro_dietiker’s picture

Title: Introduce a DiffLayout plugin or service » Introduce a DiffLayout plugin
Priority: Normal » Critical
Issue summary: View changes

Here 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.

johnchque’s picture

Assigned: Unassigned » johnchque

OK, then I think I gonna try this.

johnchque’s picture

Title: Introduce a DiffLayout plugin » [META] Introduce a DiffLayout plugin system
Issue summary: View changes

Changed to be META.

johnchque’s picture

Issue summary: View changes

Added first step.

johnchque’s picture

Issue summary: View changes

Added some other issues that need to be done first so we can update the tests earlier.

johnchque’s picture

Issue summary: View changes

Added two new steps that seems to be nice. :)

miro_dietiker’s picture

Ah 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.

johnchque’s picture

Issue summary: View changes

One more issue. As soon as this one goes in there will be just one layout plugin left. :)

johnchque’s picture

Issue summary: View changes

Two more issues that IMHO are important too.

johnchque’s picture

Issue summary: View changes

Last issue to complete the plugin system provided by diff.

miro_dietiker’s picture

When 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.

johnchque’s picture

Issue summary: View changes

Then issues removed. I think we also need to check access.

johnchque’s picture

Status: Active » Fixed

Finally done, small followups but the implementation works. :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

markusd1984’s picture

Any 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.