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.