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