Re: [Xen-devel] [PATCH 13 of 17] docs: generate an index for the html output

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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