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

[Jesse Vincent]

> Attached.
> 
> 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?

> 
> I desperately wanted to refactor to eliminate the "if (1)" bit, but I
> don't have time to get clear on how the semi-global @levels is being
> used. Maybe later.
> 
> Cheers!
> 
> --j
> -- 
> Jim Meyer, Geek at Large                                    purp at acm.org

> --- share/html/Elements/EditCustomFieldSelect	2005-08-18 19:39:27.000000000 -0700
> +++ local/html/Elements/EditCustomFieldSelect	2005-11-16 16:22:15.000000000 -0800
> @@ -84,34 +84,36 @@
>  % my $selected;
>  % my $CFVs = $CustomField->Values;
>  % my @levels;
>  % while ($CFVs and my $value = $CFVs->Next ) {
> -%       my $category = $value->Category;
> -%       if (1) { # length $category) {
> -%           my $level = (split(/:/, $category))[0];
> -%           while (@levels) {
> -%               if ($levels[-1] eq $level) {
> -%                   undef $level;
> -%                   last;
> -%               } elsif (index($level, $levels[-1]) != 0) {
> +%     my $category = $value->Category;
> +%     if (1) { # length $category) {
> +%         my $level = (split(/:/, $category || ''))[0];
> +%         while (@levels) {
> +%             if ($levels[-1] eq $level) {
> +%                 undef $level;
> +%                 last;
> +%             } elsif (index($level, $levels[-1]) != 0) {
>          </optgroup>
> -%                   pop @levels;
> -%               } else {
> -%                   last;
> -%               }
> -%           }
> -%           if (length $level) {
> -%               push @$CategoryRef, [0+ at levels, $level];
> +%                 pop @levels;
> +%             } else {
> +%                 last;
> +%             }
> +%         }
> +%         if ($level) {
> +%             push @$CategoryRef, [0+ at levels, $level];
>          <optgroup style="padding-left: <% @levels/2 %>em" label="<%$category%>">
> -%               push @levels, $level;
> -%           }
> -%       }
> +%             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) && 
> +%         $$SelectedRef == 1) {
> +%         if (($Values && $Values->HasEntry($value->Name)) || 
> +%             ($Default && ($Default eq $value->Name))) {
> +        'SELECTED'
> +%         }
> +%     }
>              ><% $value->Name%></option>
>  % }
>  % for (@levels) {
>              </optgroup>

> _______________________________________________
> Rt-devel mailing list
> Rt-devel at lists.bestpractical.com
> http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel


--