[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] docs: autogenerate search.php



On 08/09/2012 08:17 AM, Martin Kletzander wrote:
> This patch makes search.php autogenerated from search.php.in, thus
> removing hardcoded menus, footer etc. and the search.php is added to
> .gitignore.
> 
> There is new rule added for *.php files (to make it bit less
> hardcoded) that takes *.php.code.in and injects it inside the
> generated *.php (xslt was not happy about php code in the source xml).
> ---

I will flat-out admit I've never coded in php.  However, I can at least
review the makefile portions for correctness, as well as apply and test
that the patch does what it claims (and indeed, the generated search.php
file resembles the original checked in version, with the differences
being improvements in the boilerplate now that it went through the same
code generation steps as the rest of our pages).

I did _not_ test a 'make dist' from a VPATH build; there's a slight
possibility that we may need some cleanup patches as a result, but I'm
okay with that risk.

> @@ -173,6 +178,19 @@ internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in
>  	  || { rm $(srcdir)/$@ && exit 1; }; \
>  	  else echo "missing XHTML1 DTD" ; fi ; fi
> 
> +%.php.tmp: %.php.in site.xsl page.xsl sitemap.html.in
> +	@if [ -x $(XSLTPROC) ] ; then \
> +	  echo "Generating $@"; \
> +	  name=`echo $@ | sed -e 's/.tmp//'`; \

This works, although '\.' is technically tighter than '.' for matching
the literal '.' in the file name.  But why bother with sed and `` in the
first place?  You've go make to do it for you:

name=$(@:.tmp=)

> +	  $(XSLTPROC) --stringparam pagename $$name --nonet --html \

or even skip $$name and directly inline $(@:.tmp=) here.

> +	    $(top_srcdir)/docs/site.xsl $< > $@ \
> +	    || { rm $@ && exit 1; }; fi
> +
> +%.php: %.php.tmp %.php.code.in
> +	@echo "Scripting $@"; \
> +	  sed -e '/<a id="php_placeholder"><\/a>/r '"$(srcdir)/$  code in"\

Missing space before \, for consistent style.

ACK with those minor tweaks.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]