[pmwiki-devel] Patch: Improvement to Trailso we

[Ed W]

Petko Yotov wrote:
> On Friday 03 April 2009 11:29:13 Ed W wrote:
>   
>>> Just letting you know that you can easily override the default trails.php
>>> script like this :
>>> * move your modified file from scripts/ to cookbook/
>>> * add to config.php :
>>>   $EnableWikiTrails = 0;
>>>   include_once("$FarmD/cookbook/trails.php");
>>>       
>> Sure - that's exactly what I did to create the file I mailed
>>
>> However, the new file should have identical functionality to before - it
>>     
>
> The point of using your modified file is that it provides functionality 
> different from the default one, and you can upgrade to newer versions of 
> PmWiki without losing that functionality.
>   

Right - and as I said - I *already* do this for exactly that reason.  
However, hopefully it's a shortlived thing before the changes filter 
upstream for the next release...  (they are hardly controversial so I 
hope you will have few problems including them)


>> The main new functionality is that:
>>     
> [...]
>
> As I said, I haven't yet had the chance to review the changes, and I'll need 
> to find some time. In the meantime, you might add this feature request to 
> PITS, so we don't lose track of it, and so we could estimate its popularity.
>   

OK, but the changes are so small you could probably actually review them 
in less time than replying to this email.

Essentially I broke the function up into two functions.  The entry point 
function simplifies to:

    function MakeTrailStop($pagename,$trailname) {
      $links = FindNextPrev($pagename,$trailname);
      $prev=$links['prevl'];
      $next=$links['nextl'];
      return "<span class='wikitrail'>&lt;&lt; $prev | $trailname |
    $next &gt;&gt;</span>";
    }


The new function is identical to the old, except it includes these extra 
lines (in two places, slightly different for the $next)


        # Skim forward looking for next link in the same sub-tree level
        for($j=$i+1;$j<count($t);$j++) {
          if ($t[$j]['parent'] == $t[$i]['parent']) {
            $results['nextl'] = $t[$j]['markup'];
            break;
          }
          if (!$t[$j]['parent']) break; // optimisation - bailout if we 
hit top level
        }

So the logic is simply to look for another element which has the same 
parent as this element

This new code is not used in the current wiki, but it makes it simple to 
use it.  Here is my implementation:

    // Our customised version
    function MakeTrailStopMaS($pagename,$trailname) {
      $links = FindNextPrev($pagename,$trailname);
      $prev=$links['prevl'];
      $next=$links['nextl'];
      if ($prev) $prev = '&lt;&lt; '.$prev;
      if ($next) $next = $next.' &gt;&gt;';
      if ($next || $prev) {
        return "<span class='wikitrail'><span class='prev'>$prev</span>
    | <span class='next'>$next</span></span>";
      } else {
        return "";
      }
    }


Note my implementation always has a span, it can be styled to have the 
arrows inside the next/prev links using CSS, it also check that there is 
either a prev or next link to avoid leaving a dangling "|" on it's own

I didn't change any of this in the current implementation, however, I 
think it would be good to put on the todo list...

>
>   
>>>> Change the default trail separator to something more standard, eg "»"
>>>> (it looks like >> for those viewing in plain text):
>>>>     SDV($TrailPathSep, ' &raquo; ');
>>>>         
>>> I disagree. Guillemets (angle quotes, &raquo;) are indicators for quoted
>>> speech. We should use arrows (&rarr;) if we want to indicate arrows. :-)
>>> http://en.wikipedia.org/wiki/%E2%86%92
>>>       
>> I disagree your disagree...
>>
>> Wikipedia agrees that they are "sometimes used to indicate direction" -
>> however, I acknowledge that I had not realised that were used for quoted
>> speech (just an ignorant english speaker I guess...)
>>
>> A quick search of the internet suggests that for english speaking
>> websites the character I suggested is very popular and certainly way
>> more popular than the "pipe" character.
>>     
>
> What is popular is not always right. If a quick search of the internet 
> suggested that "invalid" HTML is "very popular and certainly way more popular" 
> than "valid" HTML, I wouldn't argue for a core change in that direction.
>   

I think you are being deliberately obstruse.  I'm not suggesting 
anything "invalid" is useful. 

The current pipe character is suboptimal in my opinion.  Some kind of 
arrow would be an improvement.  Whilst the raquo may not be perfect it 
seems like a good choice without going for a graphic.  After all the ">" 
greater than sign has other meanings in english and yet most people have 
no confusion when it's used also as an arrow - I am not a french 
speaker, but I doubt a symbol which looks like a small ">>" and is 
widely (incorrectly) used as a graphic on major websites will cause too 
much confusion in general

Prepared to be wrong, but in any case, some kind of arrow seems a lot 
better than a pipe character.


> Jacob Nielsen, an expert in web usability, recommends using ">" although his 
> own site uses an arrow (.GIF picture).
>   http://www.useit.com/alertbox/breadcrumbs.html
>   

Personally I don't like his arrow, I tend to agree with his words that 
">" is nicer...

> BTW, our ^|breadcrumbs|^ implementation violates the rule not to have links to 
> the current page (selflinks).
>   

Agreed - this is part of what makes me think this was an accidental 
mis-implementation that was intended to be improved later?  Original 
author here to comment?

Untested, but you could ammend my function FindPrevNext to include the 
parent in the results:

Change:

    if ($t[$i]['pagename']==$pagename) {
      # Found our page

To:

    if ($t[$i]['pagename']==$pagename) {
      # Found our page
      if ($t[$i]['parent']) $results['parent'] = $t[ $t[$i]['parent'] 
]['markup']'

This would then give you the option to use the "parent" markup as the 
center of the breadcrumb.

> I agree that this could be improved -- and should be, without breaking 
> existing wikis upon an upgrade.
>   

My changes merely give the wiki author the option to make these changes 
- the current implementation should be identical to the old

That said I personally (separately) recommend that we DO break existing 
wikis with these sensible changes to the separator and wrapping in a span...

> Arrows or ">" are fine with me; I'd use ">" for better compatibility with 
> (all) browsers. But yet, should we change a default setting if some existing 
> sites may rely on it? Generally, PmWiki doesn't do this.
>   

Backwards compatibility is only needed up to a point.  In this case 
don't have strong feeling because it doesn't affect my wiki.  However, I 
would recommend the span tag change since it's low impact.  The change 
in pipe to arrow is a "good thing" (tm) in my opinion, but obviously 
larger impact.


Good luck

Ed W
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.pmichaud.com/pipermail/pmwiki-devel/attachments/20090406/ef1c5ca8/attachment.html