Under contract for LifeWire.com/About.com I have modified the existing Diff module to add the ability to view revision deltas in an inline format rather than the 2 column format provided in current Diff module.

The Inline format is similar to how MS Word shows document changes by stroking out deletions (and highlighting in red) and showing additions in brackets (and highlighting in green).

The module requires the PEAR Text_Diff class to be installed. Not exactly sure of "Drupal's Best Practice" in this area; but for our use we decided to not include the class directly in the module by let PHP find it in standard include path as PEAR is designed. One of the PEAR files (inline.php) is included however as I needed to modify it slightly.

This module also adds/fixes the following:

- I am pretty sure there is a bug in existing module (http://drupal.org/node/196883) that prevents std node tabs from showing - i have fixed that

- i provide a user profile selectable setting to chose which diff format to use (2 column or inline) and a link to this on the diff page view.

- i remove html tags from diff output (for both formats)

- enabling module will auto-disable the diff module if it is enabled

I am including this "new module" as a ZIP rather than a patch since it will take a bit of work for me to create the patch. But, if there is interest and moshe thinks it would be good to commit this code then I could likely do up a patch.

Peter Lindstrom
LiquidCMS - Content Management Solution Experts

CommentFileSizeAuthor
lifewire_diff.zip54.86 KBliquidcms

Comments

dww’s picture

dww
we/he/they
commented

A) you can still attach multiple files to an issue or comment, so there's no need for a .zip just because you're attaching multiple files from scratch.

B) There's prior art on stripping HTML tags from diff output. See http://drupal.org/node/117688 and http://drupal.org/node/125494

C) Assuming it's not too complicated or too much code, I suspect a choice of 2-column vs. inline would be a nice feature for diff.module, but it really depends on what it involves.

D) Re: http://drupal.org/node/196883 -- please post a patch to that issue to fix that problem.

Thanks,
-Derek

liquidcms’s picture

liquidcms commented

a - but why would i want to attach multiple files when i can simply attach one zip file :)

b - took a look and it sounds like i am basically doing the same thing - replace br and p's with LF's - this was a pretty simple and minor part of my overall work here so not too bothered that i missed rotzi's previous work

c - well i wouldn't say it is simple; i use the PEAR Text_Diff class (but don't include it in new module; i let users install as part of PHP install)
- there is of course all the pieces for things like adding selection to user profile, etc
- all in all the additional code is likely not that much; but i think main reason i didn't do as a patch is because for project i am working on it was necessary to roll it out with existing diff module in place - for that reason i had to rename overlapping functions plus i added _enable hook code to disable original diff module - none of this would be required if this were to be rolled into current diff module
- another reason was that i am not too familiar with creating multi-file patches - i needed to include one new file (from the Text_Diff class) for this solution.
- i guess bottom line is i have done the code work to add what i think is a pretty good improvement to this module and my client has funded the work - hopefully some energetic developer might contribute by pulling the code out and adding it to whatever latest mix of patches that you pointed me to are (although i realize that isn't too likely). Failing all this I may try to get around to this at some point but would need to go through and do all the patches you suggest first to see if they work sufficiently and then i could just add the Inline diff engine part to that. Maybe if someone lets me know that the html issue has been rolled into cvs; i could look at adding just the Inline part of this solution to that.

d - fixing the tabs is pretty simple.. i'll post a patch for this or possibly just the code snippet (patches are useful; but certainly a bit of a pain since people rarely know what they patch against). I get that module designers like to get patches - but not sure why just giving the 5 lines of code (that i can do in 30 seconds) doesn't work. That way you can simply paste into what ever solution you have at the time - as opposed to me having to re-install original module and then create new file with just these edits, then create patch, and upload.. cut/paste is so much easier for small snippet like this.

cheers,
peter....

liquidcms’s picture

liquidcms commented

I have posted missing tab issue (http://drupal.org/node/196883) code at that node.

JASH’s picture

JASH commented

These enhancements looks quite interesting. Do you consider viable for inclusion in a future version of Diff module?

liquidcms’s picture

liquidcms commented

It would likely depend on what moshe (he is diff project owner) wants to do with this. Since i never provided a patch it likely isn't of too much use to him and it was also done again the Dr5 version. Not sure what version of this would need a patch created against it to get this included.

I'd like to think i have the time to clean this up and create a patch but i likely don't. Would be great if someone could help out; moshe contributed this very useful module and i have coded a pretty important addition to it - maybe someone can jump in and put the 2 together.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Needs work

patches must be against D6 version. i'm interested in inline diff, but not terribly so. the requirement of PEAR class will for sure generate a stream of support requests. also, we are trying to get this module into core and that complexity will not help.

liquidcms’s picture

liquidcms commented

unfortunate to hear; but i guess that means my version of your module will remain as contrib then.

btw, we have numerous content editors using our documentation editing system - my module provides the ability to set which version you want to use - 2 column or inline - none of them use 2 column format - i think it might come from people used to using MS Word and similar editors.

matkeane’s picture

matkeane commented

Thanks for posting this module, as just today I had a client ask about whether we could change the output display of the Diff module.

Your module installed fine, but I'm seeing one problem when working with foreign languages - the accented UTF8 characters are being replaced with '?' characters. Is this down to the included Pear file not treating strings as UTF8, or is the problem elsewhere? Would appreciate any pointers...

Update: OK, so it looks like it's not a UTF8 problem after all, as the accents are being converted (possibly by FCKeditor) and stored in the database as html entities (&eacute). The Diff module displays these as text - e.g. t&eacutel&eacutephone - which is confusing the hell out of the users, but the '?' isn't a geat improvement! Anyway, apologies for falsely accusing your module of being the culprit and thanks again for a very useful module.

Another Update: OK, so it was FCKeditor converting the UTF8 accents to html entities (which I've now deactivated in fckconfig.js) on each edit causing the problem. Accents are now showing up fine in the Diff output, and so I shall stop talking to myself!

Thanks,
Matthew

liquidcms’s picture

liquidcms commented

moshe, a couple other comments on #6.

it is possible (license wise, not sure??) to simply take the couple files used from the PEAR library and include them with the module. I actually already do that for one of them since i needed to modify 1 line in one of their files.

also, in core? Not sure why you want this to be part of core - pretty sure very few Drupal sites out there would be interested in reviewing revisions. What would you guess - maybe less than 1%?

dww’s picture

dww
we/he/they
commented

@ptalindstrom: re: inclusion in core and audience for this functionality -- you're way off. Anyone trying to build anything vaguely wiki-like with Drupal wants this. I run diff on 100% of the sites I've setup. We run it on 100% of the sites in *.drupal.org. Since core supports making revisions, it should provide a means of viewing them.

rötzi’s picture

rötzi commented

The best way would be to write an inline-diff formatter for the used diff-engine. The diff engine supports different output formats, so this should be possible. The engine is taken from Mediawiki (which took it from phpwiki), maybe someone already did an inline diff formatter for it?

rötzi’s picture

rötzi commented

The best way would be to write an inline-diff formatter for the used diff-engine. The diff engine supports different output formats, so this should be possible. The engine is taken from Mediawiki (which took it from phpwiki), maybe someone already did an inline diff formatter for it?

alansch’s picture

alansch commented

I've just installed lifewire diff and found the two-column mode to throw a bunch of errors (missing argument) at line 448 of lifewire_diff.module

Seems that line 448 got left in the code by accident during development as the $textDiff variable gets used nowhere else and the next line of code calls Diff correctly.

So, in function _lw_diff_table_body

  foreach($node_diffs as $node_diff) {
    if ($engine) {    // 2 column compare
      $textDiff = new Diff();
      $diff = new Diff($node_diff['old'], $node_diff['new']);

gets replaced by

  foreach($node_diffs as $node_diff) {
    if ($engine) {    // 2 column compare
      $diff = new Diff($node_diff['old'], $node_diff['new']);

and two-column diff display now works without throwing errors.

liquidcms’s picture

liquidcms commented

yup, thanks.. i had found that a couple weeks.. just forgot to post update...

i guess if this was a rel project that would be easier to do.. :(

liquidcms’s picture

liquidcms commented

ok, for anyone stumbling upon this thread or those already taking an interest, i have spawned a new project to provide this functionality.

Possibly someday we'll merge this with Moshe's version; but for now: http://drupal.org/project/lifewire_diff

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented

thanks liquidcms :)

Leeteq’s picture

Leeteq commented

Subscribing.

millarch’s picture

millarch commented

How hard would it be to have a v6 compatible module?

liquidcms’s picture

liquidcms commented

I haven't done too much with Drupal 6 as of yet;; just working on my first Dr6 project now. Since most of the work here is just "Standard" php code as opposed to too much in the way of Drupal API (nodeapi, form alters) there likely isn't that much work here. Problem is.. my time. I am a pretty busy guy and the work done on this module was originally funded by the NY Times. The project they use it on is unlikely to be upgraded to Drupal 6 anytime soon.

If anyone else is looking for this module for Drupal 6; they could of course get in touch with me to see about funding an upgrade. also, if anyone else wants to contribute the Drupal 6 version i will certainly include it to CVS (or just give them CVS access to the project).

philbar’s picture

philbar commented

subscribe

yhahn’s picture

yhahn commented

FYI I've just committed an inline diff patch to the D6 branch. http://drupal.org/node/324811

If anyone is interested in doing a D5 backport I would definitely review it.

ridefree’s picture

ridefree commented

would love a D5 version :D

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented
Title: Inline Diff Format Version of Module » Inline Diff backport to D5

@yhahn: This is an amazing addition. I just noticed that inline diffs is now an option in the workflow settings at admin/content/node-type/TYPE. great work!

Changing title to distinguish this issue as D5 (rather than duplicate) since #324811: Inline diff? is committed.

realityloop’s picture

realityloop commented
Status: Needs work » Closed (won't fix)