You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Niall Pemberton <ni...@gmail.com> on 2007/06/29 06:33:05 UTC
Re: svn commit: r551790 - in /struts/struts1/trunk/el/src/main: java/org/apache/strutsel/taglib/html/ELOptionTag.java resources/META-INF/tld/struts-html-el.tld
>From a visual scan, theres a few things that don't look quite right to me.
1) setStyleIdExpr - this method is now duplicated - should have been
renamed to setTitleKeyExpr when copy/pasting? Would have thought this
doesn't compile - for me I would have hoped the code had been tested
before committing, not just compiled - but not compiling at all is bad
news IMO.
2) Changes to release() method - I believe it should be calling
setTitleExpr(null) and setTitleKeyExpr(null) rather than
setTitle(null) and setTitleKey(null)
3) I believe as well as ELOptionTag - you also need to modify
ELOptionTagBeanInfo for the new attributes
IMO its also good to add examples to one of the webapps - provides a way to test
Niall
On 6/29/07, pbenedict@apache.org <pb...@apache.org> wrote:
> Author: pbenedict
> Date: Thu Jun 28 21:17:37 2007
> New Revision: 551790
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=551790
> Log:
> STR-2072: Add title and titleKey (EL version)
>
> Modified:
> struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java
> struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld
>
> Modified: struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java
> URL: http://svn.apache.org/viewvc/struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java?view=diff&rev=551790&r1=551789&r2=551790
> ==============================================================================
> --- struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java (original)
> +++ struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java Thu Jun 28 21:17:37 2007
> @@ -98,6 +98,18 @@
> private String styleIdExpr;
>
> /**
> + * Instance variable mapped to "title" tag attribute. (Mapping set in
> + * associated BeanInfo class.)
> + */
> + private String titleExpr;
> +
> + /**
> + * Instance variable mapped to "titleKey" tag attribute. (Mapping set in
> + * associated BeanInfo class.)
> + */
> + private String titleKeyExpr;
> +
> + /**
> * Instance variable mapped to "value" tag attribute. (Mapping set in
> * associated BeanInfo class.)
> */
> @@ -184,6 +196,22 @@
> }
>
> /**
> + * Getter method for "title" tag attribute. (Mapping set in associated
> + * BeanInfo class.)
> + */
> + public String getTitleExpr() {
> + return (titleExpr);
> + }
> +
> + /**
> + * Getter method for "titleKey" tag attribute. (Mapping set in associated
> + * BeanInfo class.)
> + */
> + public String getTitleKeyExpr() {
> + return (titleKeyExpr);
> + }
> +
> + /**
> * Getter method for "value" tag attribute. (Mapping set in associated
> * BeanInfo class.)
> */
> @@ -272,6 +300,22 @@
> }
>
> /**
> + * Setter method for "title" tag attribute. (Mapping set in associated
> + * BeanInfo class.)
> + */
> + public void setTitleExpr(String titleExpr) {
> + this.titleExpr = titleExpr;
> + }
> +
> + /**
> + * Setter method for "titleKey" tag attribute. (Mapping set in associated
> + * BeanInfo class.)
> + */
> + public void setStyleIdExpr(String titleKeyExpr) {
> + this.titleKeyExpr = titleKeyExpr;
> + }
> +
> + /**
> * Setter method for "value" tag attribute. (Mapping set in associated
> * BeanInfo class.)
> */
> @@ -294,6 +338,8 @@
> setStyleExpr(null);
> setStyleClassExpr(null);
> setStyleIdExpr(null);
> + setTitle(null);
> + setTitleKey(null);
> setValueExpr(null);
> }
>
> @@ -375,6 +421,18 @@
> EvalHelper.evalString("styleId", getStyleIdExpr(), this,
> pageContext)) != null) {
> setStyleId(string);
> + }
> +
> + if ((string =
> + EvalHelper.evalString("title", getTitleExpr(), this,
> + pageContext)) != null) {
> + setTitle(string);
> + }
> +
> + if ((string =
> + EvalHelper.evalString("titleKey", getTitleKeyExpr(), this,
> + pageContext)) != null) {
> + setTitleKey(string);
> }
>
> if ((string =
>
> Modified: struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld
> URL: http://svn.apache.org/viewvc/struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld?view=diff&rev=551790&r1=551789&r2=551790
> ==============================================================================
> --- struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld (original)
> +++ struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld Thu Jun 28 21:17:37 2007
> @@ -5240,6 +5240,31 @@
> </description>
> </attribute>
> <attribute>
> + <name>title</name>
> + <required>false</required>
> + <rtexprvalue>true</rtexprvalue>
> + <description>
> + <![CDATA[
> + <p>The advisory title for this element.</p>
> + <dl><dt><b>Since:</b></dt>
> + <dd>Struts 1.4</dd></dl>
> + ]]>
> + </description>
> + </attribute>
> + <attribute>
> + <name>titleKey</name>
> + <required>false</required>
> + <rtexprvalue>true</rtexprvalue>
> + <description>
> + <![CDATA[
> + <p>The message resources key for the advisory title
> + for this element.</p>
> + <dl><dt><b>Since:</b></dt>
> + <dd>Struts 1.4</dd></dl>
> + ]]>
> + </description>
> + </attribute>
> + <attribute>
> <name>value</name>
> <required>true</required>
> <rtexprvalue>true</rtexprvalue>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
Re: svn commit: r551790 - in /struts/struts1/trunk/el/src/main: java/org/apache/strutsel/taglib/html/ELOptionTag.java
resources/META-INF/tld/struts-html-el.tld
Posted by Paul Benedict <pb...@apache.org>.
Thanks. Updates coming up.
Niall Pemberton wrote:
>> From a visual scan, theres a few things that don't look quite right
>> to me.
>
> 1) setStyleIdExpr - this method is now duplicated - should have been
> renamed to setTitleKeyExpr when copy/pasting? Would have thought this
> doesn't compile - for me I would have hoped the code had been tested
> before committing, not just compiled - but not compiling at all is bad
> news IMO.
>
> 2) Changes to release() method - I believe it should be calling
> setTitleExpr(null) and setTitleKeyExpr(null) rather than
> setTitle(null) and setTitleKey(null)
>
> 3) I believe as well as ELOptionTag - you also need to modify
> ELOptionTagBeanInfo for the new attributes
>
> IMO its also good to add examples to one of the webapps - provides a
> way to test
>
> Niall
>
> On 6/29/07, pbenedict@apache.org <pb...@apache.org> wrote:
>> Author: pbenedict
>> Date: Thu Jun 28 21:17:37 2007
>> New Revision: 551790
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=551790
>> Log:
>> STR-2072: Add title and titleKey (EL version)
>>
>> Modified:
>>
>> struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java
>>
>>
>> struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld
>>
>>
>> Modified:
>> struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java
>>
>> URL:
>> http://svn.apache.org/viewvc/struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java?view=diff&rev=551790&r1=551789&r2=551790
>>
>> ==============================================================================
>>
>> ---
>> struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java
>> (original)
>> +++
>> struts/struts1/trunk/el/src/main/java/org/apache/strutsel/taglib/html/ELOptionTag.java
>> Thu Jun 28 21:17:37 2007
>> @@ -98,6 +98,18 @@
>> private String styleIdExpr;
>>
>> /**
>> + * Instance variable mapped to "title" tag attribute. (Mapping
>> set in
>> + * associated BeanInfo class.)
>> + */
>> + private String titleExpr;
>> +
>> + /**
>> + * Instance variable mapped to "titleKey" tag attribute.
>> (Mapping set in
>> + * associated BeanInfo class.)
>> + */
>> + private String titleKeyExpr;
>> +
>> + /**
>> * Instance variable mapped to "value" tag attribute. (Mapping
>> set in
>> * associated BeanInfo class.)
>> */
>> @@ -184,6 +196,22 @@
>> }
>>
>> /**
>> + * Getter method for "title" tag attribute. (Mapping set in
>> associated
>> + * BeanInfo class.)
>> + */
>> + public String getTitleExpr() {
>> + return (titleExpr);
>> + }
>> +
>> + /**
>> + * Getter method for "titleKey" tag attribute. (Mapping set in
>> associated
>> + * BeanInfo class.)
>> + */
>> + public String getTitleKeyExpr() {
>> + return (titleKeyExpr);
>> + }
>> +
>> + /**
>> * Getter method for "value" tag attribute. (Mapping set in
>> associated
>> * BeanInfo class.)
>> */
>> @@ -272,6 +300,22 @@
>> }
>>
>> /**
>> + * Setter method for "title" tag attribute. (Mapping set in
>> associated
>> + * BeanInfo class.)
>> + */
>> + public void setTitleExpr(String titleExpr) {
>> + this.titleExpr = titleExpr;
>> + }
>> +
>> + /**
>> + * Setter method for "titleKey" tag attribute. (Mapping set in
>> associated
>> + * BeanInfo class.)
>> + */
>> + public void setStyleIdExpr(String titleKeyExpr) {
>> + this.titleKeyExpr = titleKeyExpr;
>> + }
>> +
>> + /**
>> * Setter method for "value" tag attribute. (Mapping set in
>> associated
>> * BeanInfo class.)
>> */
>> @@ -294,6 +338,8 @@
>> setStyleExpr(null);
>> setStyleClassExpr(null);
>> setStyleIdExpr(null);
>> + setTitle(null);
>> + setTitleKey(null);
>> setValueExpr(null);
>> }
>>
>> @@ -375,6 +421,18 @@
>> EvalHelper.evalString("styleId", getStyleIdExpr(),
>> this,
>> pageContext)) != null) {
>> setStyleId(string);
>> + }
>> +
>> + if ((string =
>> + EvalHelper.evalString("title", getTitleExpr(), this,
>> + pageContext)) != null) {
>> + setTitle(string);
>> + }
>> +
>> + if ((string =
>> + EvalHelper.evalString("titleKey", getTitleKeyExpr(),
>> this,
>> + pageContext)) != null) {
>> + setTitleKey(string);
>> }
>>
>> if ((string =
>>
>> Modified:
>> struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld
>>
>> URL:
>> http://svn.apache.org/viewvc/struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld?view=diff&rev=551790&r1=551789&r2=551790
>>
>> ==============================================================================
>>
>> ---
>> struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld
>> (original)
>> +++
>> struts/struts1/trunk/el/src/main/resources/META-INF/tld/struts-html-el.tld
>> Thu Jun 28 21:17:37 2007
>> @@ -5240,6 +5240,31 @@
>> </description>
>> </attribute>
>> <attribute>
>> + <name>title</name>
>> + <required>false</required>
>> + <rtexprvalue>true</rtexprvalue>
>> + <description>
>> + <![CDATA[
>> + <p>The advisory title for this element.</p>
>> + <dl><dt><b>Since:</b></dt>
>> + <dd>Struts 1.4</dd></dl>
>> + ]]>
>> + </description>
>> + </attribute>
>> + <attribute>
>> + <name>titleKey</name>
>> + <required>false</required>
>> + <rtexprvalue>true</rtexprvalue>
>> + <description>
>> + <![CDATA[
>> + <p>The message resources key for the advisory title
>> + for this element.</p>
>> + <dl><dt><b>Since:</b></dt>
>> + <dd>Struts 1.4</dd></dl>
>> + ]]>
>> + </description>
>> + </attribute>
>> + <attribute>
>> <name>value</name>
>> <required>true</required>
>> <rtexprvalue>true</rtexprvalue>
>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org