ikiwiki (3.20130711) unstable; urgency=low
[ikiwiki.git] / 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\///