joeyh/ikiwiki — pull request #14 by jcflack (2014-10-19 21:34:07+00:00)
Title: Let plugins influence what environment variables a wrapper will preserve.

I created this patch in advance of writing the [signinview](https://ikiwiki.info/plugins/contrib/signinview/) plugin. This patch does nothing `signinview`-specific, but simply refactors wrapper generation a bit so that plugins have some influence over what environment variables a wrapper will preserve.

For example, Wrapper.pm previously hardcoded not only (some of) the RFC 3875 variables needed for a CGI wrapper (and hardcoded its own test for _whether it was generating_ a CGI wrapper), but also the Apache ErrorDocument-specific variables needed by the [404](https://ikiwiki.info/plugins/404/) plugin. Given that `signinview`, as a `403` handler, would have similar requirements to `404`, and it seemed possible other wrappers for other purposes could rely on other environment variables too, it seemed to make sense to move the preserved-variable list out of Wrapper.pm hardcoding, and closer to the plugins or other code relying on the variables.

It also seemed worthwhile, after moving the CGI-related variable list to CGI.pm, to complete the set so all the RFC 3875-specified variables were preserved, in case tricky-to-diagnose problems could result down the road with certain expected variables erased.

It is arguable that this patch would represent a slight bump (to the minor-number part) of the plugin API version. That is, it won't break any existing code, but any new code that is written to the convention established here (naming environment variables in $config{needenvkeys}) won't work in an ikiwiki version without this patch. I did not increment the number, but arguably that should happen.

If it is ok to increment the minor version, it would probably also make sense to take the several lines of open code added to 404.pm (as `registerenvkeys`) and make a new plugin-API function that does that, as suggested (with more details) in that commit comment.

Although this seemed like a worthwhile reorganization, it isn't strictly necessary for `signinview` - I could make that plugin work without it. So if this patch is objectionable for some reason, I could just rebase `signinview` to work without it.