Re: TagExtraInfo for sling:adaptTo/ tag
Hi Dan I agree with your fix to getProperty. That makes a lot of sense. I'm not sure about the fix for the adaptTo tag to prevent compilation failure. Java classes will not compile, if they reference a class that cannot be found. Why should JSPs behave differently? IMO there is value in failing early (i.e. compilation), because most likely the code is broken in any case, if the class cannot be found. Is there any real-world code leveraging this feature of the adaptTo tag? Otherwise I would lean towards adjusting the test case. In any case, I can live with either solution. Regards Julian On Thu, Jun 12, 2014 at 9:22 PM, Daniel Klco dk...@apache.org wrote: As discussed in SLING-3664 the new TEI classes are breaking the integration tests and change the behavior of some of the tags, specifically: - In adaptTo it causes the JSP to fail to compile when specifying an invalid class instead of just returning null as is the usual Sling behavior - in getProperty it causes the JSP to fail to compile if neither a defaultValue nor a returnClass are specified If we cannot provide backwards compatibly for these issues, I suggest we at least remove the TEI classes for these tags. Thanks, Dan On Thu, Apr 24, 2014 at 8:36 AM, Julian Sedding jsedd...@gmail.com wrote: Updated the patch in SLING-3475 to provide TEI implementations for all tags in the Sling taglib. Regards Julian On Wed, Apr 23, 2014 at 5:03 PM, Konrad Windszus konra...@gmx.de wrote: I definitely agree here. Including the TEI is almost no risk but it helps a lot during development (at least with IntelliJ, because Eclipse still lacks decent EL code completion support) On 23 Apr 2014, at 16:52, Julian Sedding jsedd...@gmail.com wrote: Hi all It would be great to get the patch for SLING-3475 applied soon. The added TagExtraInfo should be low risk, as it should not impact run-time behavior at all. However, enabling my IDE to help prevent small bugs would be very helpful. Especially in conjunction with Sling Models, which make the sling:adaptTo/ tag much more useful. Thanks regards Julian https://issues.apache.org/jira/browse/SLING-3475
Re: TagExtraInfo for sling:adaptTo/ tag
Good point re:adaptTo Julian. I am not sure there is a case where this would happen, but some sort of failure would make sense in the case where the code is missing. Let me investigate the options for exception handling e.g. on compilation vs runtime to see if there are any gotchas, specifically around what happens if the class is injected into OSGI after the JSP is first compiled. -Dan On Fri, Jun 13, 2014 at 5:31 AM, Julian Sedding jsedd...@gmail.com wrote: Hi Dan I agree with your fix to getProperty. That makes a lot of sense. I'm not sure about the fix for the adaptTo tag to prevent compilation failure. Java classes will not compile, if they reference a class that cannot be found. Why should JSPs behave differently? IMO there is value in failing early (i.e. compilation), because most likely the code is broken in any case, if the class cannot be found. Is there any real-world code leveraging this feature of the adaptTo tag? Otherwise I would lean towards adjusting the test case. In any case, I can live with either solution. Regards Julian On Thu, Jun 12, 2014 at 9:22 PM, Daniel Klco dk...@apache.org wrote: As discussed in SLING-3664 the new TEI classes are breaking the integration tests and change the behavior of some of the tags, specifically: - In adaptTo it causes the JSP to fail to compile when specifying an invalid class instead of just returning null as is the usual Sling behavior - in getProperty it causes the JSP to fail to compile if neither a defaultValue nor a returnClass are specified If we cannot provide backwards compatibly for these issues, I suggest we at least remove the TEI classes for these tags. Thanks, Dan On Thu, Apr 24, 2014 at 8:36 AM, Julian Sedding jsedd...@gmail.com wrote: Updated the patch in SLING-3475 to provide TEI implementations for all tags in the Sling taglib. Regards Julian On Wed, Apr 23, 2014 at 5:03 PM, Konrad Windszus konra...@gmx.de wrote: I definitely agree here. Including the TEI is almost no risk but it helps a lot during development (at least with IntelliJ, because Eclipse still lacks decent EL code completion support) On 23 Apr 2014, at 16:52, Julian Sedding jsedd...@gmail.com wrote: Hi all It would be great to get the patch for SLING-3475 applied soon. The added TagExtraInfo should be low risk, as it should not impact run-time behavior at all. However, enabling my IDE to help prevent small bugs would be very helpful. Especially in conjunction with Sling Models, which make the sling:adaptTo/ tag much more useful. Thanks regards Julian https://issues.apache.org/jira/browse/SLING-3475
Re: TagExtraInfo for sling:adaptTo/ tag
As discussed in SLING-3664 the new TEI classes are breaking the integration tests and change the behavior of some of the tags, specifically: - In adaptTo it causes the JSP to fail to compile when specifying an invalid class instead of just returning null as is the usual Sling behavior - in getProperty it causes the JSP to fail to compile if neither a defaultValue nor a returnClass are specified If we cannot provide backwards compatibly for these issues, I suggest we at least remove the TEI classes for these tags. Thanks, Dan On Thu, Apr 24, 2014 at 8:36 AM, Julian Sedding jsedd...@gmail.com wrote: Updated the patch in SLING-3475 to provide TEI implementations for all tags in the Sling taglib. Regards Julian On Wed, Apr 23, 2014 at 5:03 PM, Konrad Windszus konra...@gmx.de wrote: I definitely agree here. Including the TEI is almost no risk but it helps a lot during development (at least with IntelliJ, because Eclipse still lacks decent EL code completion support) On 23 Apr 2014, at 16:52, Julian Sedding jsedd...@gmail.com wrote: Hi all It would be great to get the patch for SLING-3475 applied soon. The added TagExtraInfo should be low risk, as it should not impact run-time behavior at all. However, enabling my IDE to help prevent small bugs would be very helpful. Especially in conjunction with Sling Models, which make the sling:adaptTo/ tag much more useful. Thanks regards Julian https://issues.apache.org/jira/browse/SLING-3475
Re: TagExtraInfo for sling:adaptTo/ tag
Updated the patch in SLING-3475 to provide TEI implementations for all tags in the Sling taglib. Regards Julian On Wed, Apr 23, 2014 at 5:03 PM, Konrad Windszus konra...@gmx.de wrote: I definitely agree here. Including the TEI is almost no risk but it helps a lot during development (at least with IntelliJ, because Eclipse still lacks decent EL code completion support) On 23 Apr 2014, at 16:52, Julian Sedding jsedd...@gmail.com wrote: Hi all It would be great to get the patch for SLING-3475 applied soon. The added TagExtraInfo should be low risk, as it should not impact run-time behavior at all. However, enabling my IDE to help prevent small bugs would be very helpful. Especially in conjunction with Sling Models, which make the sling:adaptTo/ tag much more useful. Thanks regards Julian https://issues.apache.org/jira/browse/SLING-3475
TagExtraInfo for sling:adaptTo/ tag
Hi all It would be great to get the patch for SLING-3475 applied soon. The added TagExtraInfo should be low risk, as it should not impact run-time behavior at all. However, enabling my IDE to help prevent small bugs would be very helpful. Especially in conjunction with Sling Models, which make the sling:adaptTo/ tag much more useful. Thanks regards Julian https://issues.apache.org/jira/browse/SLING-3475
Re: TagExtraInfo for sling:adaptTo/ tag
I definitely agree here. Including the TEI is almost no risk but it helps a lot during development (at least with IntelliJ, because Eclipse still lacks decent EL code completion support) On 23 Apr 2014, at 16:52, Julian Sedding jsedd...@gmail.com wrote: Hi all It would be great to get the patch for SLING-3475 applied soon. The added TagExtraInfo should be low risk, as it should not impact run-time behavior at all. However, enabling my IDE to help prevent small bugs would be very helpful. Especially in conjunction with Sling Models, which make the sling:adaptTo/ tag much more useful. Thanks regards Julian https://issues.apache.org/jira/browse/SLING-3475
