Diff Module 4.7 compatibility

Are you sure that you have made any "revisions" of the actual content? The Diff-module can not compare changes between "normal edits", but can only compare differens revisions. To create a new revision when editing some content you have to check the "Create new revision" checkbox under publishing options.

May this be your problem?
- Samuel

Sorry. Reading your post again I see that you mention "revisions", so I suppose you already know what I was telling you...

Glad this thread showed up, as I was having the same difficulty--the distinction between "edits" and "revisions" is not readily apparent.

That raises a question, though. My purpose for using this diff module is to provide wiki abilities for authenticated users, and that requires the ability to roll back or revert ANY AND ALL changes made, if necessary.

How can we set things up so that ALL changes, however minor, qualify as "revisions" and allow the ability to revert to previous versions?

Curtis

you control that via admin/settings => content types.

To set REVISIONS on by default:

ADMINISTRATER -> CONTENT -> CONFIGURE (tab) -> CONTENT TYPES (tab) -> Then click on a content type, such as: PERSONAL BLOG ENTRY. Select "Create new revision" under "Default options:"

I have looked into the code, and the reason the diff tab doesn't show is not because people don't have revisions.

In 4.7 beta 3 the revisions system is different to that in 4.6 and the module is attempting to access the obsolete $node->revisions member, which is no longer meaningful. The code sees this as empty and so believes there are no revisions.

I believe the correct way to do this under 4.7 is with node_revision_list($node), which returns a list of revisions.

I have got part way to creating a patch, but I don't have time to finish it right now, but I will look at it if I have time - depends how involved the use of the actual revisions list is in the diff code - if it's confined to the .module it should be trivial.

Just so you all know, this is not user or config error, it's a real bug created by real changes in the API, and will need a real code fix.

I've attached a replacement diff module that updates it to work with the 4.7 API. It fixes the display of the diff tab, the display of the list of revisions and the calling of the actual diff routine with the correct version ids. More puzzling, the old and new file order appeared to be back to front in the old module code, which is also corrected.

There appears to be a problem with the inline renderer. After puzzling over this for a while, I realised that the problem probably only shows in Firefox because of its rigorous application of the HTML standard. The diff module is using ins/del tags as if they are indifferent to block or inline usage, but this is not correct. If you open one of these tags within an inline context, introducing block content, such as a P tag, is illegal. IE and Opera don't care about this and render the diff pages in a usable fashion but Firefox chokes. I have not produced a fix for this problem, diff remains broken with Firefox. The workaround is to change the inline.php to put different HTML markup for the inserts and deletes, strong and em seem to work ok.

Some problems remain with this module in that the code for the 'pretty' diff renderer is not very good (it's a hack to the traditional diff renderer that takes too many shortcuts). It can't handle the case of exact same files correctly (shows an empty page rather than the file unchanged), and as mentioned above produces non-standard HTML. Additionally, the way the renderer interacts with the diff code itself creates the impression of a change set that is far from the minimal one, and is otherwise unsatisfactory. When you use the traditional diff renderer you can see these problems originate in the pretty renderer and not in the diff itself.

e.g.

File 1:
a b c
d e f
g h i

File 2:
a b c
g h i

Output:
a bDEL-START c
d e f
DEL-ENDADD_START
c

g h iADD_END

That is to say it's displaying a delete and an add operation where there is in fact just a delete operation. It looks confusing, and it is. It also seems to insert some spurious newlines, but perhaps that's just the browser screaming at the misuse of the tags, I haven' t checked.

thanks for the investigation, cat. any chance you can attach a diff here instead of a new module? also, have you checked with the PEAR project to see if they have improved their renderer? I don't want to maintain a fork at all. if you do, perhaps you want to take over this project. i'm happy to hand it off to you.

I am hoping someone can help with this error I get when trying to use Diff 4.6.0 on Drupal 4.6.5.

warning: array_merge() [function.array-merge]: Argument #1 is not an array in c:\Inetpub\wwwroot\drupal\includes\menu.inc on line 351.
warning: Missing argument 1 for diff_page() in c:\Inetpub\wwwroot\drupal\modules\diff\diff.module on line 61.
warning: Invalid argument supplied for foreach() in c:\Inetpub\wwwroot\drupal\modules\diff\diff.module on line 89.

I am running Windows 2003 Server with IIS and PHP 5. As far as I can tell my Drupal site runs as expected, and when I create revisions of documents they are listed under the diff tab, for example there will be several revisions and one has CURRENT and the others have PREVIOUS, FIRST CURRENT. When I click on any of these links I get the above errors.

Thanks.

This is a patch for the diff.module to bring to the same code level at provided here with the added array() for the callback arguements.

Paddy.

The diff.module provided by CatInTheHat works for my Drupal 4.7 installation. Didn't test the patch yet.
The released Diff Module still isn't working with 4.7. Is there a way I could apply the patch to the CVS...?

getting there

- the patch should adhere to our coding guidelines. functions are declared all on 1 line. this is wrong:
+function diff_menu($may_cache)
+{

- don't include all the PEAR stuff on every page view. there is a reason why it is in the revision_detail funcion
- please include css via drupal_add_style()

Based on these discussions, is there a new diff.module available yet? I'm using catinthehat's module and it works - but the buggy module currently in the Downloads section should be replaced.

Based on these discussions, is there a new diff.module available yet? I'm using catinthehat's module and it works - but the buggy module currently in the Downloads section should be replaced.

howdy,

can anyone who tested it tell how is the status of this patch as far as today? Does it the job and is almost RTBC?

any update apreciated!

thx

I am interested in the status of this update too

i am hoping that someone will implement the suggestions i gave in my last patch review in this issue. then i will commit.

Ok, I think I have managed to clean it up enough. I didn't use drupal_add_style(), I followed the lullabot article[1].

I am quite new to making patches, and the last one I made was backwards (- where + should have been) so forgive me if it's wrong. I have tested it on 4.7 CVS (as per the latest tarball) and it worked fine.

Thoughts?

[1] http://www.lullabot.com/articles/how_to_properly_add_css_files

Sorry, just updating the status

there are some wonky bugs in the patch, based on the fact that the order of the node revisions stuff was reversed between 4.6 and 4.7. i'll be rolling a new patch (and got the go-ahead from moshe to commit as soon as i'm ready). while i'm at it, i'm going to fix some of the crazy UI problems to make it slightly easier to use this module. ;) so, hopefully in the next few days i'll have this wrapped up, committed, and i'll create the DRUPAL-4-7 branch for this module. stay tuned...

Really looking forward to seeing this working nicely for 4.7. Allowing users to create and edit collaborative documents is a central feature of a site I have to deliver relatively soon... Revisions are good but alone are not really usable enough. Maybe diff should make its way into core eventually?

Dear dww, I'm in the same boat as caligaba--eager to deploy this feature. Anything I can do to help? I'm new to Drupal programming, but I did a lot of hacks on some other CMS (cough mambo cough).

I'm using this patch and when comparing versions inserted strings show as struck through (and red) while deleted as underlined (and green). Is it how it's supposed to be? How can I change that?

Hello Natalie,

Check that you are selecting the right source and destination files when you Diff. As Diff is showing you the changes from the one file to the other, depending on which file is selected as the source will affect the way the additions and deletions are shown, eg: are they additions to the one file or additions to the other file.

To see this, select the current file and Diff on the previous revision. Now select the previous revision and Diff on the current file. You should see the opposite behaviour in each instance.

If you want to change the colors and strikeout, make modifications to the Diff.css file (located in Diff.module)

Bye.

Hello,

I am not very good at creating Patch files, but this file is the Diff Module that has been modified to work with the node Draft module http://drupal.org/node/48731.

It calls the current document Published, any future documents Draft and any previous documents Archive. Also, you cannot Diff the current document to itself, only to previous or next.

Each document can be Diff'd against the previous, next or current document. I have tested the logic in this module and it appears to work Ok, of course if any programmers out there want to improve it, please do so.

Thanks.

Hello,

For anyone who is interested, this is the updated Node.module that goes with the previous Diff.module.

These modifications and discussions can be found in the Draft topic http://drupal.org/node/48731 and the modules currently work Ok on 4.7.x upto 4.7.3

This module gives the ability to create a new revision of a document that is then posted as a draft, leaving the original document on the web site and giving view to the draft document under the revisions and Diff modules. Suitably permissioned users can make changes to the draft document or publish the draft document as the published document.

Bye.

Hi Kieg,
These modules sound really exciting. The updated node.module works on my system, but it can't find these files in diff.module:

include_once 'Text/Diff.php';
include_once 'Text/Diff/Renderer.php';
include_once 'Text/Diff/Renderer/inline.php';

Is there something else I have to download and install that contains these php files? Are they already on my system, but Drupal is just looking for them in the wrong place?

(I'm running Drupal on Dreamhost. PHP is version 5.)

Please forgive my ignorance... I'm still new to this.

webchick is on a kick to get this done, so i'm unassigning. here's the unfinished patch from my previous attempt at this (definitely still needs work). i just never had a chance to work on this more....

+1

installed CatInTheHat module at #9 (looked the most reliable from comments above) and seems to work pretty fine here on 4.7.2

Unfortunately, if i theme my content type with phptemplate, i cannot see diffs anymore, but just title and terms of my nodes.....could anyone please help with this? is there some particular attentions i should put in my node-content-type.tpl.php files? :o

I installed the current diff module (HEAD), applied dww patch, removed the dprint_r functions and everything seems to work fine.

I also checked whether there was a new PEAR version for the diff lib: http://pear.php.net/package/Text_Diff/download/ but there were no remarkable changes. I will stick to the old one.

Here's a patch against 4.6 as of 2006-Nov-13 to update it to 4.7

I did only the minimal changes needed. No new features or anything.

Patch #35 works great here, but I had to fix css inclusion line to by using theme_add_style() otherwise the path to diff.css won't be correct.

Attached the same #35 patch but with the fix.

i'll try to take a look at this in the near future, if possible.

a) thanks for taking another stab at this crazy issue. it's sad it's gone this long without proper 4.7.x support. ;)

b) patch needs work:

1. this isn't they way to see if a node has multiple revisions or not: if ($node->vid > 1) { in fact, that's going to prevent the diff tab from showing up on node 1 if no revisions have been made. please take a closer look at how the {node_revisions} table interacts with {node} in 4.7.x.

   take a look at node.module's approach for adding the revisions tab:

   ... && db_result(db_query('SELECT COUNT(vid) FROM {node_revisions} WHERE nid = %d', arg(1))) > 1
   

2. this patch suffers from the exact same problems i was talking about up in comment #24. please (re)-read that. as it is now, the UI from this patch is really confusing, since the order is backwards from the revisions tab.

new approach. ;) start from the (now working, and basically happy) 5.x version from HEAD, and backport it to the 4.7.x API. tiny patch accomplishes this.

Ok, +1 for this new approach! I will review it soon.

FYI: thanks to http://drupal.org/node/110857 diff.module is about to radically change for the better. it'll be a little more work, but i think it'd still just backport whatever comes out of that to 4.7.x, instead of messing with the current HEAD backport i've posted here. stay tuned to that issue.

Try that one:

http://tschannen.net/diff-4.7-070130.tar.gz

Since there is no 'hook_requirements' the weight is not adjusted automatically at the moment. Thus it may be necessary to do this by hand.

I ported the last version of dww to 4.7:

http://tschannen.net/diff-4.7-070130-dww.tar.gz

(It's not on the drupal.org domain, but it works ;)

i committed rötzi's new module to HEAD, and branched that for DRUPAL-4-7. then, i committed his initial backport to the DRUPAL-4-7 branch (http://drupal.org/cvs?commit=53993). finally, i fixed up the remaining problems with the 4.7.x port (http://drupal.org/cvs?commit=53994).

so, the DRUPAL-4-7 branch is hereby open for use. ;) as soon as i get confirmation it's working for other people, i'll make an official 4.7.x-1.0 release for this.

Great! After checking your new module rötzi, I tried to backport it, but I didn't succeed. I thought it wouldn't be possible.
I'll install it and report my findings.

Thank you both (rötzi & dww) for the nice work!

It works perfect! I'll submit an issue when I find something. For now, let's close this ticket.