[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 13 of 17] docs: generate an index for the html output
On Thu, 2011-11-24 at 17:42 +0000, Ian Jackson wrote: > Ian Campbell writes ("[Xen-devel] [PATCH 13 of 17] docs: generate an index > for the html output"): > > docs: generate an index for the html output > > > > nb: I'm not a Perl wizard... > > The Perl looks reasonable in general, except that some of the style is > a bit odd. perlstyle(1) is mostly a good guide. Thanks for the thorough review. I've trimmed everything I agree with and don't have a comment on. > > + $l =~ s/.(html)$//g; > > Why the capturing parens ? I had intended to add "|txt" etc at some point. I suppose this ought to be (?:...) instead though. > > > + if ( $title eq "" ) > > + { > > Brace should be on the same line. > > > + $title = $h1 = "Xen Documentation"; > > And indent should be 4, not 1. (Multiple occurrences.) emacs seems to default to indentations of 4 spaces but unhelpfully turns two lots of that into a hard tab. I'll figure out how to nix that behaviour. > > + s/#.*$//; > > This of course prevents link anchor texts in the inde including #, > which is probably an error which it would be nice to sort out now > rather than in the future when we'll have to read this script to make > it cope. Looks like (in the suggested code below) requiring the # to be in the first column instead is your preferred solution here? Works for me. > Also you probably meant to anchor the pattern. I would do something > like: > > s/^\s+//; > s/\s+$//; > next if m/^\#/; > next unless m/\S/; I think that would be a syntax error so die unless m/\S/ ? > m/^(\S+)\s+(\S.*)$/ or die; > > > +for (@docs) { s,^${outdir}/,, } > > This is not correct because $outdir is not a regular expression. The > shortest way of doing this is indeed substr. OK. Aside: how does one dynamically construct a regex then? > Do we really want an index per subdirectory ? I was thinking of folks how manually type urls or who string the last element off. Having an index in each dir ensures they get something structured and not the apache generated thing. It does complicate the code though so I could be convinced to drop it. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |