Christopher Lenz wrote:
> Am 20.01.2007 um 09:36 schrieb Christian Boos:
>
>> Christopher Lenz wrote:
>>
>>> I'm not yet entirely sure I like a number of aspects of the design.
>>> Some of my concerns are actually about the wiki-context stuff that
>>> trac.resource is based upon... I need to catch up a bit, here.
>>>
>> Then be sure to read:
>>
>> - http://trac.edgewall.org/wiki/WikiContext, for the design of the
>> Context w.r.t. wiki processing. Basically, it's a way to keep all the
>> information needed to generate correct output in one place.
>>
>
> Okay, I've been looking into the changes, so here's some first
> feedback/questions...
>
Before going into the details, let me clarify a bit more the overall design.
Initially, the main point of the Wiki context was to be able to generate
proper Wiki content.
So there are actually two "levels" or notions of context:
- the context to which the wiki source belongs. This was needed for
properly locating relative links.
- the context of access to that text. This was needed to be able to
generate proper links.
Then, as things went on, the two sides proved equally useful, and saw
further uses. In the security branch, the "resource context" part
evolved so that there are now a few methods to let the resources
describe themselves (by the way of subclasses, when appropriate), and
the "access context" part was improved to act as a proxy for permission
requests.
One could indeed think about splitting the two notions.
But let's first go through your questions.
> Why is it possible to have a context instance not associated with any
> resource?
That's needed for creating a top-level access context.
Creating sub-contexts from such a toplevel context makes it easy (and
transparent) to create the proper subclass for the more specific context
(security branch only).
Plus, a toplevel context is a convenient way to encapsulate the env, req
and db parameters that are scattered through the code.
> Why can a context be associated with just a realm but no
> particular resourse? Or: shouldn't a context always be connected to a
> resource
This makes it easier to create a context for a specific resource in that
realm, i.e.
context = parentcontext('attachment')
for filename in ...:
ctx = context(id=filename)
Another situation is when you want to refer to some general context
which doesn't have clearly an "id" attached to it, for example:
ctxt('query'). The id _could_ be the query string itself, but right now
we don't really care. But the knowledge that we're in a sub-context of
some 'query' can be useful (think about a TicketQuery with description,
called from within a ticket description...).
> self_href() is an *awful* name. Even after reading the docstring for
> a couple of times, I still don't really get why it's there and what
> it does. Looking at the invocations, there's apparently only *one*
> place that actually passes an argument to it, and that's in
> formatter.py.
Agreed, I already wanted to change that a few times. 'resource_href'
would probably be more appropriate.
self_href (or resource_href) is used to produce the "canonical" URL for
accessing the resource that the context describes.
Also, I know the implementation is a bit messy, as it is now. I'll
simplify that.
> I can see how you chose the name because the
> Context.href() slot is already occupied; but why do we need a
> "global" href object attached to the context?
>
This is related to the access context part of the context, and the
possibility to set the context as absolute or relative (abs_urls=True).
When one wants to produce an href, one would use context.href and it
will be either a relative or an absolute href, as appropriate. This is
similar to the use of formatter.href, but we don't always have a
formatter at hand.
> Why is local_url() defined on the Context class? It's only used by
> formatter.py, so why not keep it there? It's just about how external
> links are rendered, it shouldn't concern anyone outside the formatter.
>
Yes, I originally thought it could have more uses, but finally it
didn't, so I think we can put that back in the formatter.
> What's the reason for moving the DB connection handle from the
> formatter to the context?
>
Because you often use a Context without a Formatter object, but when you
use the formatter, you always have a Context.
> Re: Context.__call__, is there really ever a need to *copy* a
> Context? I.e. why not just return `self`?
Well, it's precisely when we don't want to modify self.
Say you have a Context for a Wiki page, ctx = context('wiki', 'ThePage'),
then you create v1 = ctx(version=1), the Context for the version 1 of
that page.
You expect that ctx still refers to the latest version of the page, not
that it suddenly points to version 1...
The __call__ way makes it clear that it's a functional style. It can
also be seen as very similar to a constructor call (and actually it is
very similar).
> Talking about __call__(), I
> think code creating sub-contexts isn't all that readable. Using a
> different, more explicit way to create a subcontext might be better.
>
Well, you could always add an alias for it (subcontext? but then it's
not always a subcontext...), but I personally find the __call__ notation
to be very clear, like in, e.g. att = maincontext('wiki',
'WikiStart')('attachment', 'filename.txt').
> And a more fundamental kind of question: is it really necessary that
> the Context knows the req (and abs_urls, by extension)? I.e. why must
> a Context be able to produce links? Couldn't we just have continued
> to pass the req to e.g. render_macro() in addition to the context? I
> feel like I'm missing a fundamental point here.
>
I think so. How would a macro know that it's called from within a RSS
output or an regular HTML page? IOW, how would it know that it has to
use req.abs_href or req.href? Before, we used to check for format ==
'rss' everywhere, but that was more a hack... Now macros can use
context.href (or formatter.href, but it's the same).
>
> My general feeling is that the Context class has too much
> responsibility. When we talked about the design before you started
> working on it, *my* impression was that a wiki-context should
> encapsulate information about the resource to which the text-being-
> wikified belongs. Your idea was obviously different: "A Wiki
> rendering context contains all the information needed for properly
> rendering Wiki text". That's a whole lot broader.
>
Right, the two aspects are related, as I explained in the beginning. In
order to properly render a wiki text, one must know:
- *how* the text is accessed (the req part) in order to be able to
generate correct links and
- *where* that text belong (i.e. the resource) in order to correctly
locate relative references
See also the mail answering eblot's question "Context in a wiki macro",
where I said
In a way, the Context is a kind of "normalization" of the
request, a way to /specify/ what resource is accessed and how it is
accessed.
The req part is even more fundamental in the security branch, as
depending on *who* accesses the wiki text, the rendering can be
completely different (restrictions ranging from no access at all, to
omitting a few "private" event from the timeline or search results, or
to make some ticket links look like "missing" ones and censuring their
summary).
> I'm not saying that the design is "wrong", but in general I prefer
> functions/classes that have clear focus and responsibility. Trying to
> grok the scope of the wiki-context changes today, I couldn't help but
> feel that it introduced a certain level of "spaghettification". So
> I'd like to understand more about *why* the design looks what it
> looks like.
>
Well, I'm sure that things can still be improved further, and I'm
willing to do so, of course.
I agree that there's at least *two* notions that are addressed by the
Context, one being the AccessContext part, the other being the
ResourceDescriptor part. Maybe it's possible to separate them, maybe
not, I feel like they are intimately linked. There are two faces on a
coin, you don't necessarily want to separate them because of that ;)
The introduction of the Context as it stands now has already helped to
solve numerous issues, and its advanced use in the security branch feels
very natural. It has already offered an good deal of simplification of
the code and at the same time provided the fine grained permission
scheme we were looking for.
>> One aspect
>> that I still want to refine are the various wiki_to_ methods, which
>> should be made more flexible.
>>
>
> How so?
>
Well, using the wiki_to_... methods, we hardcode (only) 4 different ways
to format or process the wiki text.
Say we want to add a text wiki renderer (for the notification e-mails),
we'd need to add a wiki_to_text method.
I'd like to have something more modular, but don't know exactly how at
this point (e.g. it could well be a WikiText(context, fieldname, text)
object, which will do the parsing once and could be given to different
formatter objects...).
Also, a context should probably also know about the mime-type which is
currently being rendered (think about a context.wiki_to_html() call
within a template, that template being rendered by a mimeview wiki
processor ... how would the formatter now about the requested mime-type?).
Well, at least those are a few open points about which I'm still thinking.
-- Christian
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
