I've said it before and I'll say it again: AddOutputFilterByType is fundamentally unsatisfactory. This confusion is an effect, not cause.
Suffice to say, I disagree.
* Configuration is inconsistent with other filter directives. The relationship with [Set|Add|Remove]OutputFilter is utterly unintuitive and, from a user POV, broken.
I think it's really clear from the user's perspective. I think the problem comes in on the developer's side.
* Tying it to ap_set_content_type is, to say the least, hairy. IMO we shouldn't *require* modules to call this, and it's utterly unreasonable to expect that it will never be called more than once for a request, given the number of modules that might take an interest. Especially when subrequests and internal redirects may be involved.
We have *always* mandated that ap_set_content_type() should be called rather than setting r->content_type. (I wish we could remove content_type from request_rec instead.)
* It's a complexity just waiting for modules to break on it.
Anything that depends upon content-type like this is going to be hairy because there may be several 'right' answers during the course of the request.
I've made some more updates to mod_filter since I last posted on the subject, and I'm getting some very positive feedback from real users. For 2.2 I'd like to remove AddOutputFilterByType entirely, replacing it with mod_filter.
I've yet to see a clear and concise statement as to how mod_filter will solve this problem in a better and more efficient way. (Especially from a user's perspective, but also from a developer's perspective.)
mod_filter can also obsolete [Set|Add|Remove]OutputFilter, though I'm in no hurry to do that. What I can also do is re-implement all the outputfilter directives within mod_filter and its updated framework.
I will also comment that I looked in the mod_filter code the other day and was disappointed that it doesn't follow our coding style at all or even have comments that help people understand what it is trying to do inside the .c file. This all makes it very difficult to understand the code. I'd greatly appreciate it if mod_filter (and the code that you inserted elsewhere - i.e. in util_filter.c) would conform to our style guidelines and had some comments inside of it that say what it does (or trying to do).
For example, some of the things it does just makes no sense at all. filter_bucket_type() is completely bogus and needs to be tossed. The type->name field in the bucket should be used instead.
I'm getting annoyed by people doing massive code drops (i.e. mod_filter, mod_proxy, mod_auth_ldap, etc.) that don't conform to our code style and have no comments. It makes it much harder to go and fix bugs in 'em. -- justin
