You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Simon Lessard <si...@gmail.com> on 2007/07/19 14:49:40 UTC
[Trinidad] Add logging on exception in CoreRenderer
Hello all,
Do you think it would be good to add a try-catch block to CoreRenderer's
encodeEnd method that would catch RuntimeException as well as IOException to
log the message before rethrowing them again? For EL errors you almost
always get a decent stack trace. However, for other error I always seem to
end up with a .dispatch() error swallowing the real cause and obviously what
renderer failed. Right now I have to place a try-catch in the encodeAll
method of all my custom components. Am I the only one with that problem?
Regards,
~ Simon
Re: [Trinidad] [API] New Skin APIs
Posted by Simon Lessard <si...@gmail.com>.
I like it, it will also make it easier to maintain/extend if we ever add
more to a skin than a style sheet and a bundle for a reason or another.
On 7/19/07, Jeanne Waldman <je...@oracle.com> wrote:
>
> Hi there,
>
> I have some new Skin API proposals I would like to run by everyone. This
> involves public API changes and some private IMPL changes, and comes up
> while I (with code reviews by Blake Sullivan) am trying to fix JIRA
> issue https://issues.apache.org/jira/browse/TRINIDAD-105 "enable
> registerResourceBundle to any skin". (currently we get severe errors
> when running custom components against the simple skin because the
> resource bundle keys do not exist in the simple skin).
>
> Our Skin API already has the method registerStyleSheetName, so I was
> going to add a registerResourceBundleName method.
> But I have to admit that these apis really confuse me because we also
> have a getStyleSheetName API. But that doesn't get the skin-addition
> stylesheets, that gets the Skin's original stylesheet. I think we need
> to differentiate these two things better.
>
> The registerStyleSheetName API is meant for skin-additions (In
> trinidad-skins.xml, custom component developers use skin-additions to
> add a stylesheet -- and with this JIRA fixed also a resource bundle --
> to any skin, most likely the simple skin).
>
> I think that registerStyleSheetName should be something like
> addSkinAdditionStyleSheetName. And the registerResourceBundleName method
> I wanted to add would be addSkinAdditionResourceBundle. This would be a
> big improvement.
>
> But now that we will have two skin-addition properties on the Skin
> (stylesheetname and resourcebundlename), it seems clearer to add a new
> SkinAddition object to a Skin instead of having these two skin addition
> methods. A SkinAddition object can take
> styleSheetName/resourceBundleName in its constructor.
>
> [Summary for Skin.java]
>
> replace registerStyleSheetName with addSkinAddition
> add getSkinAdditions
>
> [Summary for new SkinAddition object:]
>
> Constructor takes styleSheetName and resourceBundleName
> add getStyleSheetName
> add getResourceBundleName
>
> [Changes to SkinImpl object]
>
> remove setStyleSheetName (move to constructor)
> remove setBundleName (move to constructor)
> Add the above properties to the constructor. This will clean up the API
> and prevent someone from changing the stylesheet and bundle on a Skin
> Change getBundleName to getResourceBundleName
>
>
> Thanks for your feedback!
>
> - Jeanne
>
>
>
>
>
>
>
>
Re: [Trinidad] [API] New Skin APIs
Posted by Jeanne Waldman <je...@oracle.com>.
Simon,
That is a very good question, because it made me remember that we do
sort the skin additions before we register them.
A custom component
developer creates a trinidad-skins.xml file with his skin-addition
defined that specifies his
css file and resource bundle for the custom components he has built.
Then Trinidad code reads in all the jar files on the classpath
that contain trinidad-skins.xml file.
There is code that exists that orders the skin-addition stylesheets
alphabetically so that
our Skin's styleSheetDocument's hashcode can be the same as another
system that has
the same jars with custom component information.
Here's the code from SkinUtils.java:
// register the META-INF skin additions.
// It should not matter what order we process skin-additions since
they should not
// depend upon one another.
// We sort them by style-sheet-name so the StyleSheetDocumentId will
be the same regardless
// of order. Portals are using the styleSheetDocumentId to determine
if the producer's skin
// matches the consumer's skin, and the order of the skin-additions
should not change this.
private static void _registerMetaInfSkinAdditions(
FacesContext fContext,
SkinFactory skinFactory,
List<SkinsNode> metaInfSkinsNodeList)
{
List<SkinAdditionNode> skinAdditionNodeList = new
ArrayList<SkinAdditionNode>();
for (SkinsNode skinsNode : metaInfSkinsNodeList)
{
skinAdditionNodeList.addAll(skinsNode.getSkinAdditionNodes());
}
Collections.sort(skinAdditionNodeList);
_registerSkinAdditions(fContext, skinFactory, skinAdditionNodeList,
true);
}
Simon Lessard wrote:
> I'd say List only if ordering is important, Collection otherwise.
>
> Regards,
>
> ~ Simon
>
> On 7/24/07, *Jeanne Waldman* < jeanne.waldman@oracle.com
> <ma...@oracle.com>> wrote:
>
> Thanks everyone. I'm almost done with the implementation.
> What should getSkinAdditions return? A Collection or a List?
> Also, SkinAddition is a public API, since it is needed by Skin.java,
> which is public.
>
> Here are the signatures/doc:
>
> /**
> * Adds a SkinAddition on this Skin. You can call this method as
> many
> times
> * as you like for the Skin, and it will add the SkinAddition to the
> list of
> * SkinAdditions.
> * However, it does not make sense to call this method more than
> once
> * with the same SkinAddition object.
> * This is meant for the skin-addition use-cases, where a custom
> component
> * developer has a style sheet and/or resource bundle for their
> custom
> * components, and they want the style sheet and/or resource bundle
> * to work for this Skin and the children Skins.
> * The stylesheets specified in the SkinAdditions will be merged
> with the
> * Skin's own styles.
> * The resource bundles specified in the SkinAdditions will be
> looked
> into
> * if the translated key is not found in the Skin's own resource
> bundle
> * during the call to getTranslatedString or getTranslatedValue.
> *
> * @param skinAddition The SkinAddition object to add to the Skin.
> * @throws NullPointerException if SkinAddition is null.
> */
> abstract public void addSkinAddition (
> SkinAddition skinAddition
> );
>
> /**
> * Gets a Collection of SkinAdditions that have been added on
> this Skin.
> * @return Collection a Collection of SkinAdditions.
> */
> abstract public Collection<SkinAddition> getSkinAdditions ();
>
>
> -- Jeanne
>
> Adam Winer wrote:
> > Looks good.
> >
> > -- Adam
> >
> >
> > On 7/19/07, Matt Cooper <matt.faces@gmail.com
> <ma...@gmail.com>> wrote:
> >> Hi Jeanne,
> >>
> >> I also like these changes.
> >>
> >> Thank you,
> >> Matt
> >>
> >>
> >> On 7/19/07, Blake Sullivan <blake.sullivan@oracle.com
> <ma...@oracle.com> > wrote:
> >> > Considering that I reviewed them, it isn't surprising that I
> like
> >> all of
> >> > these changes.
> >> >
> >> > -- Blake Sullivan
> >> >
> >> > Jeanne Waldman wrote:
> >> > > Hi there,
> >> > >
> >> > > I have some new Skin API proposals I would like to run by
> everyone.
> >> > > This involves public API changes and some private IMPL
> changes, and
> >> > > comes up while I (with code reviews by Blake Sullivan) am
> trying to
> >> > > fix JIRA issue
> >> https://issues.apache.org/jira/browse/TRINIDAD-105
> >> > > "enable registerResourceBundle to any skin". (currently we get
> >> severe
> >> > > errors when running custom components against the simple skin
> >> because
> >> > > the resource bundle keys do not exist in the simple skin).
> >> > >
> >> > > Our Skin API already has the method registerStyleSheetName,
> so I was
> >> > > going to add a registerResourceBundleName method.
> >> > > But I have to admit that these apis really confuse me
> because we
> >> also
> >> > > have a getStyleSheetName API. But that doesn't get the
> skin-addition
> >> > > stylesheets, that gets the Skin's original stylesheet. I
> think we
> >> need
> >> > > to differentiate these two things better.
> >> > >
> >> > > The registerStyleSheetName API is meant for skin-additions (In
> >> > > trinidad-skins.xml , custom component developers use
> >> skin-additions to
> >> > > add a stylesheet -- and with this JIRA fixed also a resource
> >> bundle --
> >> > > to any skin, most likely the simple skin).
> >> > >
> >> > > I think that registerStyleSheetName should be something like
> >> > > addSkinAdditionStyleSheetName. And the
> registerResourceBundleName
> >> > > method I wanted to add would be
> addSkinAdditionResourceBundle. This
> >> > > would be a big improvement.
> >> > >
> >> > > But now that we will have two skin-addition properties on
> the Skin
> >> > > (stylesheetname and resourcebundlename), it seems clearer
> to add
> >> a new
> >> > > SkinAddition object to a Skin instead of having these two skin
> >> > > addition methods. A SkinAddition object can take
> >> > > styleSheetName/resourceBundleName in its constructor.
> >> > >
> >> > > [Summary for Skin.java]
> >> > >
> >> > > replace registerStyleSheetName with addSkinAddition
> >> > > add getSkinAdditions
> >> > >
> >> > > [Summary for new SkinAddition object:]
> >> > >
> >> > > Constructor takes styleSheetName and resourceBundleName
> >> > > add getStyleSheetName
> >> > > add getResourceBundleName
> >> > >
> >> > > [Changes to SkinImpl object]
> >> > >
> >> > > remove setStyleSheetName (move to constructor)
> >> > > remove setBundleName (move to constructor)
> >> > > Add the above properties to the constructor. This will
> clean up the
> >> > > API and prevent someone from changing the stylesheet and
> bundle on a
> >> Skin
> >> > > Change getBundleName to getResourceBundleName
> >> > >
> >> > >
> >> > > Thanks for your feedback!
> >> > >
> >> > > - Jeanne
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> >
> >> >
> >>
> >>
> >
>
>
Re: [Trinidad] [API] New Skin APIs
Posted by Blake Sullivan <bl...@oracle.com>.
Simon Lessard wrote:
> I'd say List only if ordering is important, Collection otherwise.
I agree with Simon that the ordering issue is the crux of the decision.
In this case, while ordering does determine the relative priority of the
additions, the additions are not supposed to be overriding one
another.--their keys are supposed to be disjoint. Therefore, I would go
for Collection.
-- Blake Sullivan
>
> Regards,
>
> ~ Simon
>
> On 7/24/07, *Jeanne Waldman* < jeanne.waldman@oracle.com
> <ma...@oracle.com>> wrote:
>
> Thanks everyone. I'm almost done with the implementation.
> What should getSkinAdditions return? A Collection or a List?
> Also, SkinAddition is a public API, since it is needed by Skin.java,
> which is public.
>
> Here are the signatures/doc:
>
> /**
> * Adds a SkinAddition on this Skin. You can call this method as
> many
> times
> * as you like for the Skin, and it will add the SkinAddition to the
> list of
> * SkinAdditions.
> * However, it does not make sense to call this method more than
> once
> * with the same SkinAddition object.
> * This is meant for the skin-addition use-cases, where a custom
> component
> * developer has a style sheet and/or resource bundle for their
> custom
> * components, and they want the style sheet and/or resource bundle
> * to work for this Skin and the children Skins.
> * The stylesheets specified in the SkinAdditions will be merged
> with the
> * Skin's own styles.
> * The resource bundles specified in the SkinAdditions will be
> looked
> into
> * if the translated key is not found in the Skin's own resource
> bundle
> * during the call to getTranslatedString or getTranslatedValue.
> *
> * @param skinAddition The SkinAddition object to add to the Skin.
> * @throws NullPointerException if SkinAddition is null.
> */
> abstract public void addSkinAddition (
> SkinAddition skinAddition
> );
>
> /**
> * Gets a Collection of SkinAdditions that have been added on
> this Skin.
> * @return Collection a Collection of SkinAdditions.
> */
> abstract public Collection<SkinAddition> getSkinAdditions ();
>
>
> -- Jeanne
>
> Adam Winer wrote:
> > Looks good.
> >
> > -- Adam
> >
> >
> > On 7/19/07, Matt Cooper <matt.faces@gmail.com
> <ma...@gmail.com>> wrote:
> >> Hi Jeanne,
> >>
> >> I also like these changes.
> >>
> >> Thank you,
> >> Matt
> >>
> >>
> >> On 7/19/07, Blake Sullivan <blake.sullivan@oracle.com
> <ma...@oracle.com> > wrote:
> >> > Considering that I reviewed them, it isn't surprising that I
> like
> >> all of
> >> > these changes.
> >> >
> >> > -- Blake Sullivan
> >> >
> >> > Jeanne Waldman wrote:
> >> > > Hi there,
> >> > >
> >> > > I have some new Skin API proposals I would like to run by
> everyone.
> >> > > This involves public API changes and some private IMPL
> changes, and
> >> > > comes up while I (with code reviews by Blake Sullivan) am
> trying to
> >> > > fix JIRA issue
> >> https://issues.apache.org/jira/browse/TRINIDAD-105
> >> > > "enable registerResourceBundle to any skin". (currently we get
> >> severe
> >> > > errors when running custom components against the simple skin
> >> because
> >> > > the resource bundle keys do not exist in the simple skin).
> >> > >
> >> > > Our Skin API already has the method registerStyleSheetName,
> so I was
> >> > > going to add a registerResourceBundleName method.
> >> > > But I have to admit that these apis really confuse me
> because we
> >> also
> >> > > have a getStyleSheetName API. But that doesn't get the
> skin-addition
> >> > > stylesheets, that gets the Skin's original stylesheet. I
> think we
> >> need
> >> > > to differentiate these two things better.
> >> > >
> >> > > The registerStyleSheetName API is meant for skin-additions (In
> >> > > trinidad-skins.xml , custom component developers use
> >> skin-additions to
> >> > > add a stylesheet -- and with this JIRA fixed also a resource
> >> bundle --
> >> > > to any skin, most likely the simple skin).
> >> > >
> >> > > I think that registerStyleSheetName should be something like
> >> > > addSkinAdditionStyleSheetName. And the
> registerResourceBundleName
> >> > > method I wanted to add would be
> addSkinAdditionResourceBundle. This
> >> > > would be a big improvement.
> >> > >
> >> > > But now that we will have two skin-addition properties on
> the Skin
> >> > > (stylesheetname and resourcebundlename), it seems clearer
> to add
> >> a new
> >> > > SkinAddition object to a Skin instead of having these two skin
> >> > > addition methods. A SkinAddition object can take
> >> > > styleSheetName/resourceBundleName in its constructor.
> >> > >
> >> > > [Summary for Skin.java]
> >> > >
> >> > > replace registerStyleSheetName with addSkinAddition
> >> > > add getSkinAdditions
> >> > >
> >> > > [Summary for new SkinAddition object:]
> >> > >
> >> > > Constructor takes styleSheetName and resourceBundleName
> >> > > add getStyleSheetName
> >> > > add getResourceBundleName
> >> > >
> >> > > [Changes to SkinImpl object]
> >> > >
> >> > > remove setStyleSheetName (move to constructor)
> >> > > remove setBundleName (move to constructor)
> >> > > Add the above properties to the constructor. This will
> clean up the
> >> > > API and prevent someone from changing the stylesheet and
> bundle on a
> >> Skin
> >> > > Change getBundleName to getResourceBundleName
> >> > >
> >> > >
> >> > > Thanks for your feedback!
> >> > >
> >> > > - Jeanne
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> >
> >> >
> >>
> >>
> >
>
>
Re: [Trinidad] [API] New Skin APIs
Posted by Simon Lessard <si...@gmail.com>.
I'd say List only if ordering is important, Collection otherwise.
Regards,
~ Simon
On 7/24/07, Jeanne Waldman <je...@oracle.com> wrote:
>
> Thanks everyone. I'm almost done with the implementation.
> What should getSkinAdditions return? A Collection or a List?
> Also, SkinAddition is a public API, since it is needed by Skin.java,
> which is public.
>
> Here are the signatures/doc:
>
> /**
> * Adds a SkinAddition on this Skin. You can call this method as many
> times
> * as you like for the Skin, and it will add the SkinAddition to the
> list of
> * SkinAdditions.
> * However, it does not make sense to call this method more than once
> * with the same SkinAddition object.
> * This is meant for the skin-addition use-cases, where a custom
> component
> * developer has a style sheet and/or resource bundle for their custom
> * components, and they want the style sheet and/or resource bundle
> * to work for this Skin and the children Skins.
> * The stylesheets specified in the SkinAdditions will be merged with
> the
> * Skin's own styles.
> * The resource bundles specified in the SkinAdditions will be looked
> into
> * if the translated key is not found in the Skin's own resource bundle
> * during the call to getTranslatedString or getTranslatedValue.
> *
> * @param skinAddition The SkinAddition object to add to the Skin.
> * @throws NullPointerException if SkinAddition is null.
> */
> abstract public void addSkinAddition (
> SkinAddition skinAddition
> );
>
> /**
> * Gets a Collection of SkinAdditions that have been added on this Skin.
> * @return Collection a Collection of SkinAdditions.
> */
> abstract public Collection<SkinAddition> getSkinAdditions ();
>
>
> -- Jeanne
>
> Adam Winer wrote:
> > Looks good.
> >
> > -- Adam
> >
> >
> > On 7/19/07, Matt Cooper <ma...@gmail.com> wrote:
> >> Hi Jeanne,
> >>
> >> I also like these changes.
> >>
> >> Thank you,
> >> Matt
> >>
> >>
> >> On 7/19/07, Blake Sullivan <blake.sullivan@oracle.com > wrote:
> >> > Considering that I reviewed them, it isn't surprising that I like
> >> all of
> >> > these changes.
> >> >
> >> > -- Blake Sullivan
> >> >
> >> > Jeanne Waldman wrote:
> >> > > Hi there,
> >> > >
> >> > > I have some new Skin API proposals I would like to run by everyone.
> >> > > This involves public API changes and some private IMPL changes, and
> >> > > comes up while I (with code reviews by Blake Sullivan) am trying to
> >> > > fix JIRA issue
> >> https://issues.apache.org/jira/browse/TRINIDAD-105
> >> > > "enable registerResourceBundle to any skin". (currently we get
> >> severe
> >> > > errors when running custom components against the simple skin
> >> because
> >> > > the resource bundle keys do not exist in the simple skin).
> >> > >
> >> > > Our Skin API already has the method registerStyleSheetName, so I
> was
> >> > > going to add a registerResourceBundleName method.
> >> > > But I have to admit that these apis really confuse me because we
> >> also
> >> > > have a getStyleSheetName API. But that doesn't get the
> skin-addition
> >> > > stylesheets, that gets the Skin's original stylesheet. I think we
> >> need
> >> > > to differentiate these two things better.
> >> > >
> >> > > The registerStyleSheetName API is meant for skin-additions (In
> >> > > trinidad-skins.xml , custom component developers use
> >> skin-additions to
> >> > > add a stylesheet -- and with this JIRA fixed also a resource
> >> bundle --
> >> > > to any skin, most likely the simple skin).
> >> > >
> >> > > I think that registerStyleSheetName should be something like
> >> > > addSkinAdditionStyleSheetName. And the registerResourceBundleName
> >> > > method I wanted to add would be addSkinAdditionResourceBundle. This
> >> > > would be a big improvement.
> >> > >
> >> > > But now that we will have two skin-addition properties on the Skin
> >> > > (stylesheetname and resourcebundlename), it seems clearer to add
> >> a new
> >> > > SkinAddition object to a Skin instead of having these two skin
> >> > > addition methods. A SkinAddition object can take
> >> > > styleSheetName/resourceBundleName in its constructor.
> >> > >
> >> > > [Summary for Skin.java]
> >> > >
> >> > > replace registerStyleSheetName with addSkinAddition
> >> > > add getSkinAdditions
> >> > >
> >> > > [Summary for new SkinAddition object:]
> >> > >
> >> > > Constructor takes styleSheetName and resourceBundleName
> >> > > add getStyleSheetName
> >> > > add getResourceBundleName
> >> > >
> >> > > [Changes to SkinImpl object]
> >> > >
> >> > > remove setStyleSheetName (move to constructor)
> >> > > remove setBundleName (move to constructor)
> >> > > Add the above properties to the constructor. This will clean up the
> >> > > API and prevent someone from changing the stylesheet and bundle on
> a
> >> Skin
> >> > > Change getBundleName to getResourceBundleName
> >> > >
> >> > >
> >> > > Thanks for your feedback!
> >> > >
> >> > > - Jeanne
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> > >
> >> >
> >> >
> >>
> >>
> >
>
Re: [Trinidad] [API] New Skin APIs
Posted by Jeanne Waldman <je...@oracle.com>.
Thanks everyone. I'm almost done with the implementation.
What should getSkinAdditions return? A Collection or a List?
Also, SkinAddition is a public API, since it is needed by Skin.java,
which is public.
Here are the signatures/doc:
/**
* Adds a SkinAddition on this Skin. You can call this method as many
times
* as you like for the Skin, and it will add the SkinAddition to the
list of
* SkinAdditions.
* However, it does not make sense to call this method more than once
* with the same SkinAddition object.
* This is meant for the skin-addition use-cases, where a custom
component
* developer has a style sheet and/or resource bundle for their custom
* components, and they want the style sheet and/or resource bundle
* to work for this Skin and the children Skins.
* The stylesheets specified in the SkinAdditions will be merged with the
* Skin's own styles.
* The resource bundles specified in the SkinAdditions will be looked
into
* if the translated key is not found in the Skin's own resource bundle
* during the call to getTranslatedString or getTranslatedValue.
*
* @param skinAddition The SkinAddition object to add to the Skin.
* @throws NullPointerException if SkinAddition is null.
*/
abstract public void addSkinAddition (
SkinAddition skinAddition
);
/**
* Gets a Collection of SkinAdditions that have been added on this Skin.
* @return Collection a Collection of SkinAdditions.
*/
abstract public Collection<SkinAddition> getSkinAdditions ();
-- Jeanne
Adam Winer wrote:
> Looks good.
>
> -- Adam
>
>
> On 7/19/07, Matt Cooper <ma...@gmail.com> wrote:
>> Hi Jeanne,
>>
>> I also like these changes.
>>
>> Thank you,
>> Matt
>>
>>
>> On 7/19/07, Blake Sullivan <blake.sullivan@oracle.com > wrote:
>> > Considering that I reviewed them, it isn't surprising that I like
>> all of
>> > these changes.
>> >
>> > -- Blake Sullivan
>> >
>> > Jeanne Waldman wrote:
>> > > Hi there,
>> > >
>> > > I have some new Skin API proposals I would like to run by everyone.
>> > > This involves public API changes and some private IMPL changes, and
>> > > comes up while I (with code reviews by Blake Sullivan) am trying to
>> > > fix JIRA issue
>> https://issues.apache.org/jira/browse/TRINIDAD-105
>> > > "enable registerResourceBundle to any skin". (currently we get
>> severe
>> > > errors when running custom components against the simple skin
>> because
>> > > the resource bundle keys do not exist in the simple skin).
>> > >
>> > > Our Skin API already has the method registerStyleSheetName, so I was
>> > > going to add a registerResourceBundleName method.
>> > > But I have to admit that these apis really confuse me because we
>> also
>> > > have a getStyleSheetName API. But that doesn't get the skin-addition
>> > > stylesheets, that gets the Skin's original stylesheet. I think we
>> need
>> > > to differentiate these two things better.
>> > >
>> > > The registerStyleSheetName API is meant for skin-additions (In
>> > > trinidad-skins.xml , custom component developers use
>> skin-additions to
>> > > add a stylesheet -- and with this JIRA fixed also a resource
>> bundle --
>> > > to any skin, most likely the simple skin).
>> > >
>> > > I think that registerStyleSheetName should be something like
>> > > addSkinAdditionStyleSheetName. And the registerResourceBundleName
>> > > method I wanted to add would be addSkinAdditionResourceBundle. This
>> > > would be a big improvement.
>> > >
>> > > But now that we will have two skin-addition properties on the Skin
>> > > (stylesheetname and resourcebundlename), it seems clearer to add
>> a new
>> > > SkinAddition object to a Skin instead of having these two skin
>> > > addition methods. A SkinAddition object can take
>> > > styleSheetName/resourceBundleName in its constructor.
>> > >
>> > > [Summary for Skin.java]
>> > >
>> > > replace registerStyleSheetName with addSkinAddition
>> > > add getSkinAdditions
>> > >
>> > > [Summary for new SkinAddition object:]
>> > >
>> > > Constructor takes styleSheetName and resourceBundleName
>> > > add getStyleSheetName
>> > > add getResourceBundleName
>> > >
>> > > [Changes to SkinImpl object]
>> > >
>> > > remove setStyleSheetName (move to constructor)
>> > > remove setBundleName (move to constructor)
>> > > Add the above properties to the constructor. This will clean up the
>> > > API and prevent someone from changing the stylesheet and bundle on a
>> Skin
>> > > Change getBundleName to getResourceBundleName
>> > >
>> > >
>> > > Thanks for your feedback!
>> > >
>> > > - Jeanne
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> > >
>> >
>> >
>>
>>
>
Re: [Trinidad] [API] New Skin APIs
Posted by Adam Winer <aw...@gmail.com>.
Looks good.
-- Adam
On 7/19/07, Matt Cooper <ma...@gmail.com> wrote:
> Hi Jeanne,
>
> I also like these changes.
>
> Thank you,
> Matt
>
>
> On 7/19/07, Blake Sullivan <blake.sullivan@oracle.com > wrote:
> > Considering that I reviewed them, it isn't surprising that I like all of
> > these changes.
> >
> > -- Blake Sullivan
> >
> > Jeanne Waldman wrote:
> > > Hi there,
> > >
> > > I have some new Skin API proposals I would like to run by everyone.
> > > This involves public API changes and some private IMPL changes, and
> > > comes up while I (with code reviews by Blake Sullivan) am trying to
> > > fix JIRA issue
> https://issues.apache.org/jira/browse/TRINIDAD-105
> > > "enable registerResourceBundle to any skin". (currently we get severe
> > > errors when running custom components against the simple skin because
> > > the resource bundle keys do not exist in the simple skin).
> > >
> > > Our Skin API already has the method registerStyleSheetName, so I was
> > > going to add a registerResourceBundleName method.
> > > But I have to admit that these apis really confuse me because we also
> > > have a getStyleSheetName API. But that doesn't get the skin-addition
> > > stylesheets, that gets the Skin's original stylesheet. I think we need
> > > to differentiate these two things better.
> > >
> > > The registerStyleSheetName API is meant for skin-additions (In
> > > trinidad-skins.xml , custom component developers use skin-additions to
> > > add a stylesheet -- and with this JIRA fixed also a resource bundle --
> > > to any skin, most likely the simple skin).
> > >
> > > I think that registerStyleSheetName should be something like
> > > addSkinAdditionStyleSheetName. And the registerResourceBundleName
> > > method I wanted to add would be addSkinAdditionResourceBundle. This
> > > would be a big improvement.
> > >
> > > But now that we will have two skin-addition properties on the Skin
> > > (stylesheetname and resourcebundlename), it seems clearer to add a new
> > > SkinAddition object to a Skin instead of having these two skin
> > > addition methods. A SkinAddition object can take
> > > styleSheetName/resourceBundleName in its constructor.
> > >
> > > [Summary for Skin.java]
> > >
> > > replace registerStyleSheetName with addSkinAddition
> > > add getSkinAdditions
> > >
> > > [Summary for new SkinAddition object:]
> > >
> > > Constructor takes styleSheetName and resourceBundleName
> > > add getStyleSheetName
> > > add getResourceBundleName
> > >
> > > [Changes to SkinImpl object]
> > >
> > > remove setStyleSheetName (move to constructor)
> > > remove setBundleName (move to constructor)
> > > Add the above properties to the constructor. This will clean up the
> > > API and prevent someone from changing the stylesheet and bundle on a
> Skin
> > > Change getBundleName to getResourceBundleName
> > >
> > >
> > > Thanks for your feedback!
> > >
> > > - Jeanne
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
>
>
Re: [Trinidad] [API] New Skin APIs
Posted by Matt Cooper <ma...@gmail.com>.
Hi Jeanne,
I also like these changes.
Thank you,
Matt
On 7/19/07, Blake Sullivan <bl...@oracle.com> wrote:
>
> Considering that I reviewed them, it isn't surprising that I like all of
> these changes.
>
> -- Blake Sullivan
>
> Jeanne Waldman wrote:
> > Hi there,
> >
> > I have some new Skin API proposals I would like to run by everyone.
> > This involves public API changes and some private IMPL changes, and
> > comes up while I (with code reviews by Blake Sullivan) am trying to
> > fix JIRA issue https://issues.apache.org/jira/browse/TRINIDAD-105
> > "enable registerResourceBundle to any skin". (currently we get severe
> > errors when running custom components against the simple skin because
> > the resource bundle keys do not exist in the simple skin).
> >
> > Our Skin API already has the method registerStyleSheetName, so I was
> > going to add a registerResourceBundleName method.
> > But I have to admit that these apis really confuse me because we also
> > have a getStyleSheetName API. But that doesn't get the skin-addition
> > stylesheets, that gets the Skin's original stylesheet. I think we need
> > to differentiate these two things better.
> >
> > The registerStyleSheetName API is meant for skin-additions (In
> > trinidad-skins.xml, custom component developers use skin-additions to
> > add a stylesheet -- and with this JIRA fixed also a resource bundle --
> > to any skin, most likely the simple skin).
> >
> > I think that registerStyleSheetName should be something like
> > addSkinAdditionStyleSheetName. And the registerResourceBundleName
> > method I wanted to add would be addSkinAdditionResourceBundle. This
> > would be a big improvement.
> >
> > But now that we will have two skin-addition properties on the Skin
> > (stylesheetname and resourcebundlename), it seems clearer to add a new
> > SkinAddition object to a Skin instead of having these two skin
> > addition methods. A SkinAddition object can take
> > styleSheetName/resourceBundleName in its constructor.
> >
> > [Summary for Skin.java]
> >
> > replace registerStyleSheetName with addSkinAddition
> > add getSkinAdditions
> >
> > [Summary for new SkinAddition object:]
> >
> > Constructor takes styleSheetName and resourceBundleName
> > add getStyleSheetName
> > add getResourceBundleName
> >
> > [Changes to SkinImpl object]
> >
> > remove setStyleSheetName (move to constructor)
> > remove setBundleName (move to constructor)
> > Add the above properties to the constructor. This will clean up the
> > API and prevent someone from changing the stylesheet and bundle on a
> Skin
> > Change getBundleName to getResourceBundleName
> >
> >
> > Thanks for your feedback!
> >
> > - Jeanne
> >
> >
> >
> >
> >
> >
> >
>
>
Re: [Trinidad] [API] New Skin APIs
Posted by Blake Sullivan <bl...@oracle.com>.
Considering that I reviewed them, it isn't surprising that I like all of
these changes.
-- Blake Sullivan
Jeanne Waldman wrote:
> Hi there,
>
> I have some new Skin API proposals I would like to run by everyone.
> This involves public API changes and some private IMPL changes, and
> comes up while I (with code reviews by Blake Sullivan) am trying to
> fix JIRA issue https://issues.apache.org/jira/browse/TRINIDAD-105
> "enable registerResourceBundle to any skin". (currently we get severe
> errors when running custom components against the simple skin because
> the resource bundle keys do not exist in the simple skin).
>
> Our Skin API already has the method registerStyleSheetName, so I was
> going to add a registerResourceBundleName method.
> But I have to admit that these apis really confuse me because we also
> have a getStyleSheetName API. But that doesn't get the skin-addition
> stylesheets, that gets the Skin's original stylesheet. I think we need
> to differentiate these two things better.
>
> The registerStyleSheetName API is meant for skin-additions (In
> trinidad-skins.xml, custom component developers use skin-additions to
> add a stylesheet -- and with this JIRA fixed also a resource bundle --
> to any skin, most likely the simple skin).
>
> I think that registerStyleSheetName should be something like
> addSkinAdditionStyleSheetName. And the registerResourceBundleName
> method I wanted to add would be addSkinAdditionResourceBundle. This
> would be a big improvement.
>
> But now that we will have two skin-addition properties on the Skin
> (stylesheetname and resourcebundlename), it seems clearer to add a new
> SkinAddition object to a Skin instead of having these two skin
> addition methods. A SkinAddition object can take
> styleSheetName/resourceBundleName in its constructor.
>
> [Summary for Skin.java]
>
> replace registerStyleSheetName with addSkinAddition
> add getSkinAdditions
>
> [Summary for new SkinAddition object:]
>
> Constructor takes styleSheetName and resourceBundleName
> add getStyleSheetName
> add getResourceBundleName
>
> [Changes to SkinImpl object]
>
> remove setStyleSheetName (move to constructor)
> remove setBundleName (move to constructor)
> Add the above properties to the constructor. This will clean up the
> API and prevent someone from changing the stylesheet and bundle on a Skin
> Change getBundleName to getResourceBundleName
>
>
> Thanks for your feedback!
>
> - Jeanne
>
>
>
>
>
>
>
[Trinidad] [API] New Skin APIs
Posted by Jeanne Waldman <je...@oracle.com>.
Hi there,
I have some new Skin API proposals I would like to run by everyone. This
involves public API changes and some private IMPL changes, and comes up
while I (with code reviews by Blake Sullivan) am trying to fix JIRA
issue https://issues.apache.org/jira/browse/TRINIDAD-105 "enable
registerResourceBundle to any skin". (currently we get severe errors
when running custom components against the simple skin because the
resource bundle keys do not exist in the simple skin).
Our Skin API already has the method registerStyleSheetName, so I was
going to add a registerResourceBundleName method.
But I have to admit that these apis really confuse me because we also
have a getStyleSheetName API. But that doesn't get the skin-addition
stylesheets, that gets the Skin's original stylesheet. I think we need
to differentiate these two things better.
The registerStyleSheetName API is meant for skin-additions (In
trinidad-skins.xml, custom component developers use skin-additions to
add a stylesheet -- and with this JIRA fixed also a resource bundle --
to any skin, most likely the simple skin).
I think that registerStyleSheetName should be something like
addSkinAdditionStyleSheetName. And the registerResourceBundleName method
I wanted to add would be addSkinAdditionResourceBundle. This would be a
big improvement.
But now that we will have two skin-addition properties on the Skin
(stylesheetname and resourcebundlename), it seems clearer to add a new
SkinAddition object to a Skin instead of having these two skin addition
methods. A SkinAddition object can take
styleSheetName/resourceBundleName in its constructor.
[Summary for Skin.java]
replace registerStyleSheetName with addSkinAddition
add getSkinAdditions
[Summary for new SkinAddition object:]
Constructor takes styleSheetName and resourceBundleName
add getStyleSheetName
add getResourceBundleName
[Changes to SkinImpl object]
remove setStyleSheetName (move to constructor)
remove setBundleName (move to constructor)
Add the above properties to the constructor. This will clean up the API
and prevent someone from changing the stylesheet and bundle on a Skin
Change getBundleName to getResourceBundleName
Thanks for your feedback!
- Jeanne
Re: [Trinidad] Add logging on exception in CoreRenderer
Posted by Adam Winer <aw...@gmail.com>.
Hrm, I've never had that much of a problem with dispatch()
swallowing the cause... wonder what's going wrong there...
I couldn't remember the cost of adding try/catch - I knew it's
very small, but couldn't remember how small (since
CoreRenderer gets hit a lot, I'm paranoid), but:
http://www.ujug.org/stuff/1522_performance_myths_exposed.pdf
answers that question. (Short answer: except for *very*
tight loops, esp. looping through arrays, it's essentially free.)
That said, what I'm thinking we might do is try something a
bit fancier to try and prevent the "half-HTML-half-stacktrace"
syndrome - which is especially bad in XMLHttp, where
you get "half-XML-half-stacktrace", which is as good as
no XML at all, and defeats the efforts of the PPR system
to do a good job with error cases. Have to think how that
would work, but I'm toying with a ResponseWriter extension
that can close up elements automatically and show the
stack trace in some nicer form in debug mode, or in
non-debug mode redirect to an error page. WDYT?
-- Adam
On 7/19/07, Simon Lessard <si...@gmail.com> wrote:
> Hello all,
>
> Do you think it would be good to add a try-catch block to CoreRenderer's
> encodeEnd method that would catch RuntimeException as well as IOException to
> log the message before rethrowing them again? For EL errors you almost
> always get a decent stack trace. However, for other error I always seem to
> end up with a .dispatch() error swallowing the real cause and obviously what
> renderer failed. Right now I have to place a try-catch in the encodeAll
> method of all my custom components. Am I the only one with that problem?
>
>
> Regards,
>
> ~ Simon