Page MenuHomePhabricator

Deprecate null skin on DefaultOutputTransform
Open, Needs TriagePublic


The DefaultOutputTransform introduced in allows a null skin to be passed as an option. This possibility should be deprecated and the skin (or a narrower interface thereof, see comments in the file) should always be passed as an option.

Event Timeline

*Probably* the way this should happen is that the skins should have their own post processing passes (aka subclass of OutputTransform) and instead of passing in a skin to the DefaultOutputTransform the skin mechanism will just insert it's own post processing pass *after* DefaultOutputTransform.

Roughly, everyplace that calls

$po = $services->getDefaultOutputTransform()->transform($po, [ 'skin'=>....]);

will eventually be migrated to:

$po = $services->getDefaultOutputTransform()->transform($po, [ ....]);
$po = $skin->getTransform()->transform($po);

Need some thought, since I think the current idea is to hide the default output transform inside ParserOutputAccess, more or less. From the Graph extension work (Ie39cbf2e9c5c4d5a5ba910f83e2287c332314b8c) there was a proposal to make a new "Null" skin (similar to, but distinct from, the existing "Api" skin), and it would make sense to use that as a default if no skin is passed, so that we effectively *always* have /some/ skin in the pipeline.

But the bigger lift would be adding the Skin to the ParserOptions, more or less along the lines of I719c115194059060f7f888608417a194ac80cc92, which would probably mean serializing it if it turns out to split the cache. And that's actually fine as well, you'd just serialize the skin as the *name* of the skin, presumably. But getting all the bits in place might be nontrivial.