[Rt-devel] PATCH: silence warnings Elements/EditCustomFieldSelect

[Jim Meyer]

Hello again!

On Wed, 2005-11-16 at 17:40, Jim Meyer wrote:
> On Wed, 2005-11-16 at 16:55, Jesse Vincent wrote:
> > > While there's a bit of a refactor involved toward the end, this diff is
> > > largely just a rework of the indents to be consistent (which makes it
> > > easier to refactor =). I pulled a bunch of "... && ... && ... &&" stuff
> > > out of a <%%> element around the 'SELECTED' line and collapsed the logic
> > > a bit.
> > 
> > It's very hard to see what you actually changed with the big indent
> > change. Could you do this as two layered patches?
> 
> Happily so, and I've corrected the subject's misplaced location, too.
> 
> Attached are the indent revision (EditCustomFieldSelect.indent.patch)
> and the silencing of the warnings (EditCustomFieldSelect.fixes.patch).

Please drop the previous EditCustomFieldSelect.fixes.patch and make use
of this one. The last one buggered the logic as I misread the second
term of:

<%$Values->HasEntry($value->Name) && ($$SelectedRef = 1) && 'SELECTED'%>

... as a condition (==) rather than assignment; little wonder I didn't
much like the original logic. I also left the single-quotes on SELECTED
while making it an HTML literal, so the whole thing started misbehaving
on me (though it was very quiet about it ;p).

Cheers!

--j
-- 
Jim Meyer, Geek at Large                                    purp at acm.org
-------------- next part --------------
--- share/html/Elements/EditCustomFieldSelect.indents	2005-11-21 16:46:32.000000000 -0800
+++ share/html/Elements/EditCustomFieldSelect	2005-11-21 16:48:46.000000000 -0800
@@ -86,9 +86,9 @@
 % my @levels;
 % while ($CFVs and my $value = $CFVs->Next ) {
 %     my $category = $value->Category;
 %     if (1) { # length $category) {
-%         my $level = (split(/:/, $category))[0];
+%         my $level = (split(/:/, $category || ''))[0];
 %         while (@levels) {
 %             if ($levels[-1] eq $level) {
 %                 undef $level;
 %                 last;
@@ -98,20 +98,21 @@
 %             } else {
 %                 last;
 %             }
 %         }
-%         if (length $level) {
+%         if ($level) {
 %             push @$CategoryRef, [0+ at levels, $level];
         <optgroup style="padding-left: <% @levels/2 %>em" label="<%$category%>">
 %             push @levels, $level;
 %         }
 %     }
         <option value="<%$value->Name%>" 
-% if ($Values) {
-            <% $Values->HasEntry($value->Name) && ($$SelectedRef = 1) && 'SELECTED' %>
-% } elsif ($Default) {
-            <% ($Default eq $value->Name) && ($$SelectedRef = 1) && 'SELECTED' %>
-% }
+%     if ($value->Name && $SelectedRef && ref($SelectedRef) &&
+%         (($Values && $Values->HasEntry($value->Name)) || 
+%          ($Default && ($Default eq $value->Name)))) {
+%         $$SelectedRef = 1;
+          SELECTED
+%     }
             ><% $value->Name%></option>
 % }
 % for (@levels) {
             </optgroup>