mpeshev / Theme-Mentor

Theme Mentor - helper plugin for WordPress, the cousin of Theme-Check reporting other possible theme issues
21 stars 4 forks source link

Do not mix optional and required tests #1

Open thefuxia opened 11 years ago

thefuxia commented 11 years ago

When I run the test on the T5 Theme (used on wpkrauts.com) I got the following message:

No wp_footer or closing body tag found at file footer.php No wp_head or closing head tag found at file header.php

First, the or is misleading: What exactly is the problem?

Second, the elements html, body, head and tbody don’t need an explicit notation. They exist without being written in HTML. In plain English: You cannot search the written markup to see if they exist, because there is no need to write them down.

I leave those out usually, because I see no reason to waste the bandwidth. :)

My suggestion: Do not test that. Look for wp_head() and wp_footer() only. And do it right:

<?php
# wp_head();
?>
</head>

Why does this pass the test?

mpeshev commented 11 years ago

It's a valid remark, however I never intended to build Theme Mentor as an accurate and bulletproof resource. If I were to follow the standards strictly, I would have built an extension to Theme-Check which is not the case here.

Even after the first 100+ reviews on wporg themes, there are plenty of obvious guidelines that I forget to check for new themes due to the fact that there are hundreds or remarks for any theme, based on UI, UX, code quality, compatibility and so forth.

In other words, I'm trying to combine several things: 1) Maximal automation for code related issues 2) Less hassle when validation checks grow to hundreds, or thousands 3) Different validation options

Validations available so far are 'extracted' from existing theme reviews. There are more in the queue, such as reading the different text domains used (to prevent copy-paste text domains), multiple occurrences of the same ID in a file or even escaping functions not used in src/href tags for variables. Fault tolerance is a must for that purpose.

If you want to build a competent and argue-free resource, feel free to fork and extend the Theme Mentor. Practically wise I consider the end result as a 'take a look here as this looks suspicious and decide whether this is crucial or you have your own reasons to do it this way'. It's either proving your point, or correcting your typo-like issue.

P.S. Another milestone upfront includes options (like, what to test for) and ignoring previous results, but this is 0.1 and we're totally not there yet.

mpeshev commented 11 years ago

P.S. For the specific comment 'sharp' char - you know that regex is an expensive resource and I'm sacrificing punctuality to form a single regex instead of a complex conditional one or several ones. If you find regular expressions in the codebase that make sense and could be improved, I'd be happy to update.