Patch Proposal: show revisions around version

Motivation

playing with viewing different revisions, I had to modify the url manually to go to adjacent versions to the one being viewed. I checked the source and was happy to notice that right there where I wanted to modify things someone had already suggested to perform the same modification.

Description

in the block containing extra links regarding the topic being viewed it modifies the part "revisions" part. it calculates a "window" of revisions around the one being viewed and shows links to them. in case the latest revisions falls outside the calculated window, it adds a link to it, before the calculated window.

Documentation

If necessary, user documentation of any new features introduced by this patch.

Examples

viewing r14 of a topic with 66 revisions (links are bold):

Revisions: r66 | r15 | > | r14 | > | r13 | Total page history | Backlinks

Implementation

Note: a first patch is attached as ShowRevisionsAroundVersion.diff. The patch is against the TWikiAlphaRelease 3733.

an alternative patch (not incremental) ShowRevisionsAroundVersion-2.diff handles special case for uniformity

Revisions: r66 | > | r65 | > | r64 | > | r63 | Total page history | Backlinks

Revisions: r66 | r64 | > | r63 | > | r62 | Total page history | Backlinks

Discussion:

Sweet. I patched my local DevelopBranch, and would have checked it in if ntwiki wasn't dead. Looks really good

I also changed the default $cfg{NumberOfRevisions} to 4, because with the new shorter version numbers there is space, and it means that the number of older/newer revisions is balanced (2 newer and 2 older).

-- CrawfordCurrie - 06 Mar 2005

I'm glad you appreciate. please add this single line, it is necessary in case the highest revision count is lower than $cfg{NumberOfRevisions}.

     my $revsToShow = $TWiki::cfg{NumberOfRevisions};
+    $revsToShow = $maxrev if $maxrev < $revsToShow;
     my $doRev = $rev + $revsToShow / 2;

-- MarioFrasca - 06 Mar 2005

I already rewrote it a bit.

    # Show revisions around the one being displayed
    my $revsToShow = $TWiki::cfg{NumberOfRevisions} + 1;
    my $topRev = $rev + $revsToShow / 2;
    $topRev = $revsToShow if $topRev < $revsToShow;
    $topRev = $showRev if $topRev > $showRev;
    my $revisions = "";
    my @revs;
    while( $revsToShow && $topRev ) {
        if( $topRev == $rev) {
            push( @revs, "r$rev" );
        } else {
            push( @revs, "<a href=\"".
                  $session->getScriptUrl( $webName, $topicName, "view" ) .
                  "?rev=$topRev\" $TWiki::cfg{NoFollow}>r$topRev</a>" );
        }
        if( $revsToShow > 1 && $topRev > 1 ) {
            push( @revs, "<a href=\"".
                  $session->getScriptUrl( $webName, $topicName, "rdiff").
                  "?rev1=$topRev&amp;rev2=".($topRev-1)."\" $TWiki::cfg{NoFollow}>&gt;</a>" );
        }
        $revsToShow--;
        $topRev--;
    }
    $revisions = join(" ", @revs );

Note that the $cfg{NumberOfRevisions} is supposed to be the number of extra revisions not including the one being shown.

Unfortunately ntwiki is still dead, otherwise I would check in.

-- CrawfordCurrie - 06 Mar 2005

Hi Crawford, all right about the correction on $cfg{NumberOfRevisions}, the one about @revs is for efficience and I'm sure it is better, even though with sucn short lists maybe you won't really notice the difference. but good habits are good in all situations.

I don't know why you want to check against two conditions instead of one. I'd say the second will never be tested, since they look equivalent to me and the shorter, the better (easier to read for the human reader. I'm reading and I'm reading aloud: revsToShow... ah, yes, the number of versions to show, but topRev, is it the top rev, no, it is being decremented... so what does it mean actually? ... let's see ... and 2 minutes of useful human energy is lost ...). ah, I see, it is a patch to the same problem I solved by recomputing revsToShow. no, I prefer my correction also because it saves the definition of revsToShow.

don't forget the use integer

also I like being able to jump back to the current version... if there's a link anywhere else, all right, otherwise I'd rather leave the extra case.

...

well, since I was correcting a working thing, what about this one ( don't say you don't like it because I feel strongly about it ). I did not like the previous version because there was some repeated code, which I obviously hate.

    # Show revisions around the one being displayed $rev
    # we start at $maxrev then possibly jump near $rev if too distant
    my $revsToShow = $TWiki::cfg{NumberOfRevisions} + 1;
    $revsToShow = $maxrev if $maxrev < $revsToShow;
    my $doingRev = $maxrev;
    my @revs;
    while( $revsToShow > 0 ) {
        $revsToShow = $revsToShow - 1;
        if( $doingRev == $rev) {
            push( @revs, "r$rev");
        } else {
            push( @revs, "<a href=\"".
                  $session->getScriptUrl( $webName, $topicName, "view" ) .
                  "?rev=$doingRev\" $TWiki::cfg{NoFollow}>r$doingRev</a>");
        }
        if ($doingRev-$rev >= $TWiki::cfg{NumberOfRevisions}) {
            # we're too far away, need to jump closer to $rev.
            # recompute $doingRev and skip the difflink;
            use integer;
            $doingRev = $rev + $revsToShow / 2;
            next;
        }
        if( $revsToShow != 0 ) {
            push( @revs, "<a href=\"".
                  $session->getScriptUrl( $webName, $topicName, "rdiff").
                  "?rev1=$doingRev&amp;rev2=".($doingRev-1)."\" $TWiki::cfg{NoFollow}>&gt;</a>");
        }
        $doingRev = $doingRev - 1;
    }
    my $revisions = join(" | ", @revs);

-- MarioFrasca - 06 Mar 2005

Dunno where you see the repeated code... but does it really matter? It works, and it's short. I tested it against all the edge cases I could think of, including:

• less than NumberOfRevisions revs available
• rev in middle of range
• rev at top of range, within 1, 2, 3 of the top
• rev at bottom of range, within 1, 2, 3 of the bottom

Committed to SVN r3765. Many thanks, Mario!

-- CrawfordCurrie - 06 Mar 2005

Crawford, I downloaded the version from svn. it works fine but it does not include a link to the most recent version. including it might force you to repeat code. or you could use the patch I was suggesting before (updated to match svn 3765). try it and check the difference. I agree about getting rid of those pipes.

-- MarioFrasca - 06 Mar 2005

Oh, I see what you mean now. I'm not convinced that a link to the most recent rev is needed, since in the average case all the user needs to do is hit "back" and in the worst case (where they have been given a link to an older rev) all the have to do is trim the url.

-- CrawfordCurrie - 06 Mar 2005

well, actually avoiding to edit the url by hand was (almost) all what I was concerned about when I suggested this modification. ...well, I did say "almost".

one more thing about revisions: the link "topic end" jumps to the end of the latest version of the topic, not the one being displayed. maybe we could correct the template so that it only refers to the anchor (in the current page) and not to the url+anchor where the url has to be computed and when we compute things we make mistakes.

-- MarioFrasca - 07 Mar 2005

OK, looks good. Merged as r3770. Does'nt this require user documentation somwehere (I don't know, I'm asking)

-- CrawfordCurrie - 08 Mar 2005

Now the pipe separators are missing between revision numbers. Compare the Revisions numbers on the current page to that in develop. Apparently in View.pm, line 250 is not called. For now I've changed the state of this topic.

-- ArthurClemens - 25 Mar 2005

We took the pipe symbols out because (1) they use up too much real-estate and (2) they have no semantic value. Do you really object to the way it looks now?

-- CrawfordCurrie - 26 Mar 2005

No pipes gives less clutter, I agree. But the separate links now look like one link, because the underline has no gaps. What about a space as separator?

-- ArthurClemens - 26 Mar 2005

An  ? OK, that works for me. Checked in as 3901.

-- CrawfordCurrie - 26 Mar 2005

Looks better. Almost there: in the link of each revision, it looks if the r character has a preceding space.

-- ArthurClemens - 26 Mar 2005

OK, I changed it to ↔ which makes the spacing look right. If you hate it, change it back to > - it's fairly obvious where it's done (View.pm, L259)

-- CrawfordCurrie - 27 Mar 2005

Actually, I've changed it to <, which gives a better idea of direction. In line 241, I removed the space before the r: " r$doingRev" is now "r$doingRev".

-- ArthurClemens - 27 Mar 2005

Why do we need the r before each revision number at all? Wouldn't the string Revisions: suffice? Well, I guess this is just aesthetics. The one place where the r makes some sense is in the topic title shown by the browser when viewing an older revision.

-- FranzJosefSilli - 22 Jun 2005