Adding an isset() conditional to checkbox option handling is entirely appropriate, and, as far as I know, perfectly acceptable.<div><br></div><div>If I were to suggest any rewriting of the options handling, it would be to find a way to loop through input types (checkbox, select, radio, text, textarea, etc.), rather than handling each one explicitly. (And perhaps you've done this; I've not looked at the code in question.)</div>
<div><br></div><div>Chip<br><br><div class="gmail_quote">On Wed, Jun 8, 2011 at 4:07 PM, Darren Slatten <span dir="ltr"><<a href="mailto:darrenslatten@gmail.com">darrenslatten@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"><br><blockquote style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204, 204, 204);padding-left:1ex" class="gmail_quote">Anyway, that sort of question is kinda petty when you get right down<br>
to it. Theme review is an ongoing process, and the guidelines will<br>
change as needed. For now, I agree with the no-PHP-notices rule, as it<br>
shows an attention to detail. I myself don't always check my plugins<br>
to not produce notices, but it's something I know that I should do.<br></blockquote><br></div>I completely agree. However, the theme I'm working on right now throws 100+ notices in some situations. They are all due to undefined variables/indices that are used to store the theme's options. Unfortunately, the options panel only saves the option values that have been explicitly set (so for example, an option with an empty checkbox is not saved as false--it's simply left undefined). Then...throughout the entire theme, the author used a code pattern like this:<br>
<br><div style="margin-left:40px;font-family:courier new,monospace">if ( $themeslug_options['footer']['show_copyright_info'] )<br> themeslug_copyright();<br></div><br>Basically, the author is working with the assumption that <i>undefined</i> will behave just like <i>false</i> (and honestly, I can't think of a relevant scenario where this assumption would cause problems).<br>
<br>So in a case like this (keeping in mind that this theme has a huge amount of options), flushing out all the Undefined notices would require substantial changes to the code. What would be the best way to resolve this? The solutions I see are:<br>
<br><br><br>Change all occurrences of this:<br><br><div style="margin-left:40px;font-family:courier new,monospace">if ( $foo['bar'] )<br></div><br>...to this:<br><br><div style="margin-left:40px"><span style="font-family:courier new,monospace">if ( isset( $foo['bar'] ) && $foo['bar'] )</span><br>
</div><br><br><br>OR<br><br><br><br>Rewrite the options panel (~3,000 lines of code).<br><br><br><br>I'm guessing the latter solution is preferable?<br><font color="#888888"><br>-Darren</font><div><div></div><div class="h5">
<br><br><br><br><br><div class="gmail_quote">On Wed, Jun 8, 2011 at 1:56 PM, Otto <span dir="ltr"><<a href="mailto:otto@ottodestruct.com" target="_blank">otto@ottodestruct.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Wed, Jun 8, 2011 at 1:49 PM, Darren Slatten <<a href="mailto:darrenslatten@gmail.com" target="_blank">darrenslatten@gmail.com</a>> wrote:<br>
> I could be wrong, but I got the feeling that part of the question was along<br>
> the lines of:<br>
><br>
> If PHP notices are not permitted, then why do some already-accepted themes<br>
> generate PHP notices?<br>
<br>
</div>Obviously, no theme review process involving human judgment is going<br>
to be perfect.<br>
<br>
In the end, you're just going to have to say "just because theme X has<br>
the problem doesn't make it not actually a problem".<br>
<br>
Anyway, that sort of question is kinda petty when you get right down<br>
to it. Theme review is an ongoing process, and the guidelines will<br>
change as needed. For now, I agree with the no-PHP-notices rule, as it<br>
shows an attention to detail. I myself don't always check my plugins<br>
to not produce notices, but it's something I know that I should do.<br>
<div><div></div><div><br>
-Otto<br>
_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org" target="_blank">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
</div></div></blockquote></div><br>
</div></div><br>_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br></blockquote></div><br></div>