incubator-bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Koželj <pe...@digiverse.si>
Subject Re: [Whitelabeling] Quick assessment of (current) whitelabelling implementation .
Date Wed, 19 Dec 2012 10:37:23 GMT
>
> If my comment above is accurate : no need to do nothing of this . This
> is happening every day , every where in Trac
>
> Maybe I didn't understand the initial statement about option defaults .
> CMIIW
>

We were not on the same page here :( as I misunderstood  your initial
observation.
I was considering env.config.options to automatically enumerate labels but
then I had nowhere to put the defaults.
Anyway this has nothing to do with your observation and I apologize  for
confusion.

I can use trac.config.Option instead of self.env.config.get if this is the
preferred way.
So is trac.config.Option the preferred way? Is if so is that just a
convention or are there deeper technical reasons to prefer one over another?



>
> Please review the implementation of trac.web.chrome.Chrome.dispatch
> method and request callbacks . Using data may lead to clashes with
> other active request handlers
>

Reviewed, it was an useful exercise :) My conclusions:
- data needs to be dict, but of corse that does not mean it is an
appropriate holder for the labels by itself
- req.chrome is always evaluated by Chrome.render_template, but I agree
that the same should be avoided by request filter methods

So if we don't want to add it to data or force lazy evaluation of
req.chrome, the only thing that remains is a new property req.labels as:

{{{

def post_process_request(self, req, template, data, content_type):
    ...
    req.callbacks['labels'] = lambda req: self._get_whitelabelling()
    ...

}}}

This works but doing req.labels['whatever'] does leave a bit strange taste
in my mouth as it seems out of place.

Regards,
Peter

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message