Flag branches as ready for review, by using a ready/ prefix as Joey suggested
[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 [[!template id=gitbranch author="[[harishcm]]" branch=smcv/ready/harishcm-map-fix]]
68
69 Patch:
70
71     --- /usr/local/share/perl/5.8.8/IkiWiki/Plugin/map.pm
72     +++ map.pm
73     @@ -80,7 +80,17 @@
74         my $indent=0;
75         my $openli=0;
76         my $addparent="";
77     -   my $map = "<div class='map'>\n<ul>\n";
78     +   my $map = "<div class='map'>\n";
79     +
80     +   # Return empty div if %mapitems is empty
81     +   if (!scalar(keys %mapitems)) {
82     +           $map .= "</div>\n";
83     +           return $map; 
84     +   } 
85     +   else { # continue populating $map
86     +           $map .= "<ul>\n";
87     +   }
88     +
89         foreach my $item (sort keys %mapitems) {
90                 my @linktext = (length $mapitems{$item} ? (linktext => $mapitems{$item}) : ());
91                 $item=~s/^\Q$common_prefix\E\///