I've looked at it. forget about the one time performance hit at build time, I'd say.
for me, this is ok. regards, Martin On 11/24/05, Simon Kitching <[EMAIL PROTECTED]> wrote: > Thanks very much Steve. > > As this patch is a reasonably significant change, I'd like to wait at > least 24 hours for other MyFaces committers to have a look at least at > my summary of this patch (see below). If there are no objections during > that period I'll then test and commit your patch as soon as I can find > the necessary time. > > Cheers, > > Simon > > [EMAIL PROTECTED] wrote: > > I submitted 2 more files: a tar that contains the new files that were > > missing from the .patch, and a patch for tomahawk/tld/tomahawk.tld that > > brings it up to date with the first patch. > > > > Steve > > > > On 11/23/05, *Simon Kitching* <[EMAIL PROTECTED] > > <mailto:[EMAIL PROTECTED]>> wrote: > > > > Hi Steve, > > > > [EMAIL PROTECTED] <mailto:[EMAIL PROTECTED]> wrote: > > > Both tomahawk/tld/myfaces_ext.dl and tomahawk/tld/tomahawk.tld > > will need > > > some additional external entities added to pick up the standard HTML > > > stuff that I have in my patch. > > > > > > > I had a look at your patch. To summarise for others: > > > > The patch moves definitions for various html attributes out of the > > per-html-component files and into a file per html attribute to avoid > > duplicate definitions across many files. Per-html-component files (like > > "html_button_attributes.xml") then contain a reference to a > > per-attribute file for each attribute that a button supports, rather > > than containing the definition inline. > > > > Possible issue: this effectively makes it *mandatory* for a compile-time > > step to generate the tld by composing the necessary files. Up to now > > this has been done because some servlet engines don't support xml > > entity > > refs in tlds at runtime, but with one file per html attribute even those > > containers that do support xml entity refs would take a fair performance > > hit. I don't see this as a major problem myself; generating the tld by > > expanding the refs seems to work nicely but I thought I would point it > > out in case someone else objects. > > > > The patch file you attached to JIRA doesn't include those new > > per-attribute files. I obviously can't commit your patch without them, > > as otherwise this will completely break MyFaces. Can you tar or zip the > > new "entities/html_attributes" dir containing these files and attach > > that to the JIRA issue? > > > > > > > It looks from here (subversion newbie) like tomahawk/tld/entities > > and > > > impl/tld/entities refer to the same files. If so, both > > > tomahawk/tld/myfaces_ext.dl and tomahawk/tld/tomahawk.tld will > > need some > > > additional external entities added to pick up the standard HTML > > stuff > > > that I have in my patch. > > > > File tomahawk/tld/myfaces_ext.tld is auto-generated from the > > tomahawk/tld/tomahawk.tld file by the build.xml file. > > > > Re dir: tomahawk/tld/entities, yes it is an "svn externals" > > reference to > > the impl/tld/entities dir. Thanks for pointing that out, I hadn't > > spotted that. So fixing impl will clean up tomahawk as well; that's > > good. > > > > > > > > Is there a simple way for a noncommitter to test in that > > environment, or > > > should I wait to see if my patch is accepted before I submit a > > patch to > > > update the other files? That is probably simplest. > > > > I don't understand what problem you have with testing this. In order to > > emulate the "externals" behaviour, you just need to make > > tomahawk/tld/entities into a symlink to the impl/tld/entities on a unix > > platform. Or at worst, just copy the impl/tld/entities dir over the > > tomahawk/tld/entities dir if you're working on an inferior OS :-) > > > > > > > > Thanks for the patch. > > > > Regards, > > > > Simon > > > > > > -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces
