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... Why is it possible to have a context instance not associated with any resource? 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? 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. 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? 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. What's the reason for moving the DB connection handle from the formatter to the context? Re: Context.__call__, is there really ever a need to *copy* a Context? I.e. why not just return `self`? 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. 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. 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. 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. > One aspect > that I still want to refine are the various wiki_to_ methods, which > should be made more flexible. How so? > This improvement is related to the > mimeview refactoring and the wiki engine refactoring (parsing/ > formatting > split). Related how? Cheers, Chris -- Christopher Lenz cmlenz at gmx.de http://www.cmlenz.net/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
