Hi,
Thanks for fixing it right away! :)
Kind regards,
Arjan Tijms
On Tue, Jan 31, 2017 at 9:26 PM, Edward Burns <edward.burns_at_oracle.com>
wrote:
> >>>>> On Sun, 29 Jan 2017 16:50:17 +0100, arjan tijms <
> arjan.tijms_at_gmail.com> said:
>
> AT> I'm reviewing the search expression API classes a little. First of
> all, I'm
> AT> *very* impressed by the quality of the work, this is really well done!
>
> >>>>> On Sun, 29 Jan 2017 21:56:12 -0500, Leonardo Uribe <
> leonardo.uribe_at_irian.at> said:
>
> LU> Yes, it took a lot of time and a lot of iterations to get it done.
>
> Yes, that's what I thought. That argued in favor of accepting it even
> though it was uncomfortably late.
>
> AT> I did found a tiny tiny issue in the API class
> AT> javax.faces.component.search.ComponentNotFoundException though. It's
> almost
> AT> too small to mention, but for completeness: it doesn't declare a static
> AT> final serialVersionUID field.
>
> Fixed that, thanks.
>
> AT> Another small issue that I noticed is in the Javadoc of:
>
> AT> javax.faces.application.Application.addSearchKeywordResolver(
> SearchKeywordResolver)
>
> AT> It currently says:
>
> AT> /**
> AT> * <p class="changed_added_2_3">Cause an the argument
> AT> <code>resolver</code> to be added to the resolver chain.
> AT> *
> AT> * It is not possible to remove an {_at_link SearchKeywordResolver}
> AT> * registered with this method, once it has been registered.</p>
> AT> *
> AT> * <p>The default implementation throws
> AT> * <code>UnsupportedOperationException</code> and is provided
> AT> * for the sole purpose of not breaking existing applications that
> AT> extend
> AT> * {_at_link Application}.</p>
> AT> *
> AT> * @throws IllegalStateException if called after the first
> AT> * request to the {_at_link javax.faces.webapp.FacesServlet}
> has been
> AT> serviced.
> AT> * @throws NullPointerException when resolver is null.
> AT> *
> AT> * @param resolver the SearchKeywordResolver to add.
> AT> * @since 2.3
> AT> */
>
> AT> There's a small typo -> "Cause an the ..."
>
> AT> But more importantly, it doesn't say if the new SearchKeywordResolver
> is
> AT> added in front of the chain or at the end.
>
> AT> In the implementation it's very explicitly added to the front, so it
> can
> AT> override existing resolvers. Perhaps a small clarification here would
> help?
>
> LU> New SearchKeywordResolver instances must be put on top. The idea is if
> the
> LU> developer wants to override an existing keyword, it should be able to
> do it
> LU> just adding it to the chain, just like with EL resolvers.
>
> Fixed that.
>
> Note that I pushed it to the new MOJARRA_2_3X_ROLLING branch and used
> git cherry-pick to get it on master. Manfred will have more to say
> about branches shortly.
>
> Ed
>
> --
> | edward.burns_at_oracle.com | office: +1 407 458 0017
> | 25 business days until planned start of JSF 2.3 Final Approval Ballot
> | 15 business days until DevNexus 2017
> | 40 business days until JavaLand 2017
>