doc/bugs/map_fails_to_close_ul_element_for_empty_list.mdwn

   1 [[!tag plugins/map patch]]
   2
   3 input:
   4
   5     before.
   6     \[[!map pages="sdfsdfsdfsd/*"]]
   7     after.
   8
   9 Presuming that the pagespec does not match, output:
  10
  11     <p>before.
  12     <div class="map">
  13     <ul>
  14     </div></p>
  15
  16 The UL element is not closed.
  17
  18 Patch:
  19
  20     --- /usr/share/perl5/IkiWiki/Plugin/map.pm  2009-05-06 00:56:55.000000000 +0100
  21     +++ IkiWiki/Plugin/map.pm   2009-06-15 12:23:54.000000000 +0100
  22     @@ -137,11 +137,11 @@
  23             $openli=1;
  24             $parent=$item;
  25         }
  26     -   while ($indent > 0) {
  27     +   while ($indent > 1) {
  28             $indent--;
  29             $map .= "</li>\n</ul>\n";
  30         }
  31     -   $map .= "</div>\n";
  32     +   $map .= "</ul>\n</div>\n";
  33         return $map;
  34      }
  35
  36
  37 -- [[Jon]]
  38
  39 > Strictly speaking, a `<ul>` with no `<li>`s isn't valid HTML either...
  40 > could `map` instead delay emitting the first `<ul>` until it determines that
  41 > it will have at least one item? Perhaps refactoring that function into
  42 > something easier to regression-test would be useful. --[[smcv]]
  43
  44 >> You are right (just checked 4.01 DTD to confirm). I suspect refactoring
  45 >> the function would be wise. From my brief look at it to formulate the
  46 >> above I thought it was a bit icky.  I'm not a good judge of what would
  47 >> be regression-test friendly but I might have a go at reworking it. With
  48 >> this variety of problem I have a strong inclination to use HOFs like map,
  49 >> grep. - [[Jon]]
  50
  51 >>> The patch in [[map/discussion|plugins/map/discussion]] has the same
  52 >>> problem, but does suggest a simpler approach to solving it (bail out
  53 >>> early if the map has no items at all). --[[smcv]]
  54
  55 >>>> Thanks for pointing out the problem. I guess this patch should solve it.
  56 >>>> --[[harishcm]]
  57
  58 >>>>> Well, I suppose that's certainly a minimal patch for this bug :-)
  59 >>>>> I'm not the IkiWiki maintainer, but if I was, I'd apply it, so I've put
  60 >>>>> it in a git branch for Joey's convenience. Joey, Jon: any opinion?
  61 >>>>>
  62 >>>>> If you want to be credited for this patch under a name other than
  63 >>>>> "harishcm" (e.g. your real name), let me know and I'll amend the branch.
  64 >>>>> (Or, make a git branch of your own and replace the reference just below,
  65 >>>>> if you prefer.) --[[smcv]]
  66
  67 >>>>>> The current arrangement looks fine to me. Thanks. --[[harishcm]]
  68
  69 > [[merged|done]] --[[Joey]]
  70
  71 Patch:
  72
  73     --- /usr/local/share/perl/5.8.8/IkiWiki/Plugin/map.pm
  74     +++ map.pm
  75     @@ -80,7 +80,17 @@
  76         my $indent=0;
  77         my $openli=0;
  78         my $addparent="";
  79     -   my $map = "<div class='map'>\n<ul>\n";
  80     +   my $map = "<div class='map'>\n";
  81     +
  82     +   # Return empty div if %mapitems is empty
  83     +   if (!scalar(keys %mapitems)) {
  84     +           $map .= "</div>\n";
  85     +           return $map;
  86     +   }
  87     +   else { # continue populating $map
  88     +           $map .= "<ul>\n";
  89     +   }
  90     +
  91         foreach my $item (sort keys %mapitems) {
  92                 my @linktext = (length $mapitems{$item} ? (linktext => $mapitems{$item}) : ());
  93                 $item=~s/^\Q$common_prefix\E\///