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

[Martin Kletzander]

On 08/09/2012 05:00 PM, Eric Blake wrote:
> 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.
> 

I'll try that, but I won't probably find necessarily all the issues that
might occur.

>> @@ -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=)
> 

Yes, I haven't realized that when remaking other rule to suit my needs,
but it's definitely better looking, easier and faster.

All the other rules use this, so I'll see what I can do with that later
(maybe some bigger cleanup wouldn't hurt), but ...

>> +	  $(XSLTPROC) --stringparam pagename $$name --nonet --html \
> 
> or even skip $$name and directly inline $(@:.tmp=) here.
> 

... I went with this version in this patch.

>> +	    $(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.
> 

Fixed.

> ACK with those minor tweaks.
> 

Thanks, pushed.

Martin