Bug: Viewing differences between revisions 1.2 and 1.1 shows also 1.1

Example: http://twiki.org/cgi-bin/rdiff/Codev/StructureOfOndiskTopicFormat?rev1=1.2&rev2=1.1

(URL generated by TWiki running on TWiki.org, no point in filling in bug template - the template would just say "on twiki.org", "on twiki.org", "on twiki.org"...)

The above URL should just show the difference between revision 1.1 and 1.2 - ie what was added. It actually shows that and the contents of revision 1.1. It's been this way for quite a while.

NB

This only happens if the older revision being compared against is revision 1.1

Test case

http://twiki.org/cgi-bin/rdiff/Codev/StructureOfOndiskTopicFormat?rev1=1.2&rev2=1.1

Environment

-- MS - 07 Feb 2004

Follow up

As you noticed, this happens only when you compare 1.2 to 1.1. When I initially implemented the version control I just left this as a minor inconsistency (80/20 rule). This can be fixed if someone brings a patch, or we can declare this as a BugNotToBeFixed.

I do not really know what the UserUnfriendly was for in the RelatedTopics form field, so I removed it.

-- PeterThoeny - 29 Jan 2004

Fix record

The code assumes, that you want all revisions, if the lower revision is 1.1. It just needs to handle the special case, that the upper rev is 1.2. This bugged me quite a few times, just not bad enough to fix it; maybe someday...

-- PeterKlausner - 08 Feb 2004

I've put back UserUnfriendly. The reason for it is because it's a common term for when software does things that the user doesn't expect or want, be it unexpected behaviour (such as showing too many revisions or the behaviour CrawfordCurrie raised in the case of attachment management), corruption or content, restriction on what people can say/do in topics, and so on.

Like PeterKlausner I was noting this as a bug, rather than adding the fix since whilst it's a bug it's only a minor annoyance for me. It is UserUnfriendly though -- consider pulling in an rdiff output via and %INCLUDE%. (Since the number of globals in TWiki prevents true nested subrequests)

-- MS - 08 Feb 2004

the problem lies in the fact that, standing the default difftype='history', when you are asking for the differences between version x and 1 and you're not specifying the type of diff you want, you also get the "difference between 1 and 0". this is, as far as I can judge, the least surprising behaviour when handling the sequence of all changes from version 1 to any other chosen version x.

the other part of the problem lies in the fact that diffing between two adjacent versions does not include the needed the parameter type=diff. (if you include it, the problem disappears.)

my suggestion is to make 'diff' the default and to repare the templates to make them include the parameter type=history for history rdiff. the other possibility is to leave 'history' the default and repare the templates to make them include the parameter type=diff for diff rdiff.

leaving alone the changes to the templates, this is what I did to my lib/TWiki/UI/RDiff.pm:

--- ./lib/TWiki/UI/RDiff.pm   2004-08-08 10:28:45.000000000 +0200
+++ /home/intrafici/public_html/twiki/lib/TWiki/UI/RDiff.pm   2005-03-03 14:57:22.413281704 +0100
@@ -379,7 +379,7 @@
   my $rev2 = $query->param( "rev2" );
 
   $renderStyle = "sequential" if ( ! $renderStyle );
-  $diffType = "history" if ( ! $diffType );
+  $diffType = "diff" if ( ! $diffType );
   $contextLines = 3 if ( ! $contextLines );
 
   return unless TWiki::UI::webExists( $webName, $topic );
@@ -467,7 +467,7 @@
 
   # do one or more diffs
   $difftmpl = &TWiki::handleCommonTags( $difftmpl, $topic );
-  if( $topicExists ) {
+  if( ($topicExists) && ($rev1 > $rev2) ) {
     my $r1 = $rev1;
     my $r2 = $rev2;
     my $rInfo = "";
@@ -500,6 +500,10 @@
     $diff = $difftmpl;
     $diff =~ s/%REVTITLE1%/$revTitle1/go;
     $diff =~ s/%REVTITLE2%/$revTitle2/go;
+    if ($topicExists) {
+      my $rInfo1 = getRevInfo( $webName, $rev1, $topic, 1 );
+      $diff =~ s/%REVINFO1%/$rInfo1/go;
+    }
     $diff =~ s/%TEXT%//go;
     $diff =~ s/( ?) *<\/?(nop|noautolink)\/?>\n?/$1/gois;   # remove <nop> and <noautolink> tags
     print $diff;

please note the modified default value for $difftype. you may want to choose otherwise.

-- MarioFrasca - 03 Mar 2005

with 'diff' the default rdiff type, 'history' is explicitly needed.

Sending        DEVELOP/templates/rdiff.pattern.tmpl
Sending        DEVELOP/templates/view.tmpl
Transmitting file data ..
Committed revision 3851.
Sending        DEVELOP/templates/Main/view.pattern.tmpl
Sending        DEVELOP/templates/view.pattern.tmpl
Transmitting file data ..
Committed revision 3852.

-- MarioFrasca - 23 Mar 2005

Index: RDiff.pm
===================================================================
--- RDiff.pm    (revision 3853)
+++ RDiff.pm    (working copy)
@@ -350,19 +350,14 @@
     my $webName = $session->{webName};
     my $topic = $session->{topicName};
 
-    my $renderStyle = $query->param('render');
-    $renderStyle = $session->{prefs}->getPreferencesValue( 'DIFFRENDERSTYLE' ) unless ( $renderStyle );
-    my $diffType = $query->param('type');
-    my $contextLines = $query->param('context');
-    $contextLines = $session->{prefs}->getPreferencesValue( 'DIFFCONTEXTLINES' ) unless ( $contextLines );
+    my $renderStyle = $query->param('render') || $session->{prefs}->getPreferencesValue( 'DIFFRENDERSTYLE' ) || 'sequential';
+    my $diffType = $query->param('type') || 'diff';
+    my $contextLines = $query->param('context') || $session->{prefs}->getPreferencesValue( 'DIFFCONTEXTLINES' );
+    $contextLines = 3 unless defined $contextLines;
     my $skin = $session->getSkin();
     my $rev1 = $query->param( 'rev1' );
     my $rev2 = $query->param( 'rev2' );
 
-    $renderStyle = 'sequential' if ( ! $renderStyle );
-    $diffType = 'diff' if ( ! $diffType );
-    $contextLines = 3 unless ( defined $contextLines );
-
     TWiki::UI::checkWebExists( $session, $webName, $topic );
 
     my $tmpl = '';

checked in parameters handling refactoring r3857

-- WillNorris - 23 Mar 2005

OK, good - I can't reproduce the original problem, so setting this to done.

-- CrawfordCurrie - 26 Mar 2005