> broken is broken! I'm sure there will be a way to accomodate though > w/o breaking.
Maybe by just having 2 interfaces, the old one and the new one ? CloD ----- Original Message ----- From: "Geir Magnusson Jr" <[EMAIL PROTECTED]> To: "Velocity Developers List" <[EMAIL PROTECTED]> Sent: Thursday, April 15, 2004 1:41 PM Subject: Re: Event Handler patch > > On Apr 14, 2004, at 4:24 PM, Will Glass-Husain wrote: > > > Hi Geir, > > > > Thanks for the detailed comments, particularly on global/local. I > > think > > such a distinction is a good one, but I suggest we should give the > > handler > > developer some flexibility. The old way (attaching to the context) > > still > > works (almost exactly the same). The new way (velocity.properties) > > allows > > for global handlers. > > I don't quite get the meaning of 'but' here. All I did was name the > ones attached via vel.prop as 'global' and the others as 'local', > meaning global to all renderings vs local to a specific one. > > [SNIP] > > > > > Having said all this, this is a minor point. > > Yes. > > > If you or others feel that > > it's confusing to have the context as an argument in the EventHandler > > interfaces, (giving the ability to take local circumstances into > > account in > > a globally registered event handler) we can take it out. > > It makes perfect sense w/ a global handler, and a local one too. The > good thing about a local one is that the author can do it if they > choose... we wouldn't need special support for the local ones (esp if > the support broke things) > > > > > Next, regarding backwards compatibility... look forward to your > > thoughts on > > this once you review the patch. The break is very minor, just a > > change in > > the method signatures for the event handler interfaces. (For example, > > the > > handlers now require an initialize method). I'm not sure how to > > structure > > this in a backwards compatible way without a whole new interface, which > > seems redundant. > > broken is broken! I'm sure there will be a way to accomodate though > w/o breaking. > > > > > Finally, answer your questions: > >> Hm. These both then are examples of handlers w/ specific purposes? > > > > > > The new interface "IncludeEventHandler" is a class of event handlers > > (similar to ReferenceInsertionEventHandler and > > MethodExceptionEventHandler) > > that allows the developer to modify the behavior of #include and > > #parse. > > Essentially, it's what I proposed previously as InputEventHandler in > > Bug # > > 20342). I've included two specific implementations IncludeNotFound, > > that > > provides for a (configurable) page to include when the requested > > template > > can not be found, and "IncludeRelativePath", that provides for relative > > paths for includes. Similarly, I've created "EscapeXMLEntities" > > which a > > specific implementation of the ReferenceInsertionEventHandler to > > escapes > > HTML/XML entities. I think it'd be nice to include in Velocity a small > > number of handlers for commonly requested features. (developers will > > want > > to create custom handlers for more specific situations, of course). > > > >> How do you configure them? [IncludeNotFound] > > Through velocity.properties. See the Javadocs. > > > > This will probably make better sense once you have a chance to browse > > the > > patch. > > > > Best regards, WILL > > > > > > > > > > > > ----- Original Message ----- > > From: "Geir Magnusson Jr" <[EMAIL PROTECTED]> > > To: "Velocity Developers List" <[EMAIL PROTECTED]> > > Sent: Wednesday, April 14, 2004 12:15 PM > > Subject: Re: Event Handler patch > > > > > >> > >> On Apr 14, 2004, at 1:42 PM, Will Glass-Husain wrote: > >> > >>> Hi, > >>> > >>> I've just posted in Bugzilla a fairly substantial patch to the event > >>> handling facilities of Velocity. I'd love to get feedback from other > >>> Velocity developers, particularly those using event handlers. This > >>> patch is > >>> intended to make the event handlers easier to use and also more > >>> flexible. > >>> It includes a new event handler interface "IncludeEventHandler", and > >>> several > >>> pre-built handlers that can be just plugged in. > >>> > >>> BACKGROUND > >>> > >>> I created this in response to a couple of frustrations > >>> > >>> - Event handlers are currently somewhat cumbersome to configure, with > >>> special code required to "attach" them to the context. This always > >>> seemed a > >>> little odd to me, since from the user perspective this has nothing to > >>> do > >>> with the primary purpose of the context. Now the event handlers can > >>> be > >>> configured in velocity.properties. > >> > >> It certainly does - if your handler is specific to the request, then > >> the context is the right way to do it. That said, 'global' handlers > >> aren't a bad idea, but a global solution doesn't discount the need for > >> a local one. > >> > >>> > >>> - event handlers currently run in isolation, which limits their > >>> utility. > >>> They have no ability to call system services (e.g. to see if a > >>> template > >>> exists or to check a velocity property) and no ability to tell where > >>> they > >>> are on the page or what else is being processed. Now event handlers > >>> have > >>> access to RuntimeServices and to the current context. > >> > >> That's probably not a bad idea. I'm trying to remember if there were > >> any 'security' issues surrounding this, or we just didn't do it 'cuz'. > >> > >>> > >>> - There's been the occasional call on the list for modifications to > >>> the > >>> behavior of #parse and #include. For example, using retrieving files > >>> with > >>> relative paths, or displaying a "page not found" message when the > >>> template > >>> is missing. The new IncludeEventHandler allows the developer to > >>> modify the > >>> behavior of these directives. > >>> > >>> - Some developers also need HTML escaping of references (for example, > >>> this > >>> is turned on by default in JSTL). This patch provides a built in > >>> ReferenceInsertionHandler to accomplish this. > >>> > >> > >> Hm. These both then are examples of handlers w/ specific purposes? > >> > >>> > >>> SPECIFICS > >>> > >>> The specific changes are listed below. If this is too much for a > >>> single > >>> patch, let me know and we can break them apart. I've created a > >>> "readme.txt" > >>> that gives more detailed instructions. Once the patch is accepted, > >>> I'll > >>> write the docs. Geir's made a few positive comments about this > >>> approach, so > >>> I'm optimistic about eventual committer action. Regardless, I really > >>> look > >>> forward to feedback. > >>> > >>> (1) Event handlers can be registered in the velocity-properties, e.g. > >>> > >>> eventhandler.referenceinsertion.class = > >>> org.apache.velocity.app.event.implement.EscapeXMLEntities > >>> > >>> (2) Multiple event handlers can be registered for a given type and > >>> are > >>> applied in a "filter" pattern. The specifics vary per event handler > >>> (documented in the JavaDocs). The following example would first > >>> enforce a > >>> relative path rule to included templates, and would then check to see > >>> if the > >>> template is actually available. > >>> > >>> eventhandler.include.class = > >>> org.apache.velocity.app.event.implement.IncludeRelativePath,org.apach > >>> e. > >>> veloc > >>> ity.app.event.implement.IncludeNotFound > >>> > >> > >> How do you configure them? > >> > >>> (3) Event handlers have a new lifecycle. Before they are called they > >>> are > >>> initialized with a link to RuntimeServices. This allows the handlers > >>> to > >>> retrieve Velocity properties and to have access to a number of > >>> interesting > >>> system methods. Each event handler is also given the current context > >>> when > >>> it is called. This allows the template designer (or custom > >>> directives) to > >>> change the behavior of the event. A nice example of this is the > >>> EscapeXMLEntities reference insertion handler, which stops escaping > >>> HTML > >>> when an "ignoreEscape" flag is set in the context. > >> > >> Now you've come full circle and are making this context dependent - I > >> as the developer have to trust/assume that someone configured the > >> engine externally to have a EscapeEXMLEntities handler and then use it > >> via the context? > >> > >> This sounds like a nice handler, but I wonder if having a 'global' > >> handler be context dependent makes sense. > >> > >>> > >>> (4) New implemented event handlers include: > >>> -- EscapeXMLEntities (escapes HTML/XML & < > ") > >>> -- IncludeNotFound > >>> -- IncludeRelativePath > >>> -- PrintExceptions > >>> > >>> Using these built-in event handlers, users can do useful things (e.g. > >>> escape > >>> HTML) with no extra coding just by configuring velocity.properties. > >> > >> I'm pretty sure that there are cases where a engine-global hander is > >> useful, but I'd also bet there's lots of cases (escaping entities or > >> relative path stuff) where current context matters - who is logged in, > >> what the material being rendered is, etc... What portion of a page is > >> being rendered... > >> > >>> > >>> (5) The patch is almost entirely backwards compatible. The old > >>> "attach the > >>> handler to the context" still works, although this results in a > >>> per-request > >>> rather than per-application lifecycle for the handler. One > >>> difference... > >>> the API for the event handlers has changed, and as such will require > >>> minor > >>> editing and recompilation of user code. (I think this is worth it). > >>> All > >>> tests (old and new) work. > >> > >> Well, I'll dig in and look. I suspect I'll come back w/ a suggestion > >> that we don't break backwards compatibility (surprise, surprise) and > >> that maybe we expand the system to have 'global' and 'local', where > >> the > >> global are as you suggest - configurable in the velocity.properties > >> and > >> local as they are now, possibly w/ the addition of being able to > >> accept > >> runtime services. > >> > >> I see that you could have two kinds of handlers - one, the current > >> 'local' handlers which are context specific for anything where there's > >> a runtime decision or configuration involved, and the 'global' > >> external > >> type for cases where it doesn't matter - it should apply globally for > >> all renderings.... > >> > >> geir > >> > >>> > >>> Best regards, > >>> > >>> WILL > >>> _______________________________________ > >>> Forio Business Simulations > >>> > >>> Will Glass-Husain > >>> [EMAIL PROTECTED] > >>> www.forio.com > >>> > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe, e-mail: [EMAIL PROTECTED] > >>> For additional commands, e-mail: [EMAIL PROTECTED] > >>> > >>> > >> -- > >> Geir Magnusson Jr 203-247-1713(m) > >> [EMAIL PROTECTED] > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [EMAIL PROTECTED] > >> For additional commands, e-mail: [EMAIL PROTECTED] > >> > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [EMAIL PROTECTED] > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > -- > Geir Magnusson Jr 203-247-1713(m) > [EMAIL PROTECTED] > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
