Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INFO: class-tgm-plugin-activation.php The theme appears to use include or require. If these are being used to include separate sections of a template from independent files, then get_template_part() should be used instead. Line 1063: require_once( ABSPATH . 'wp-admin/includes/class-wp-list-table.php' ); #258

Closed
timnicholson opened this issue Jan 22, 2015 · 8 comments

Comments

@timnicholson
Copy link

When using the WordPress Theme Check plugin, the above warning message is generated. It hasn't prevented my themes from being accepted to the WordPress.org repository, but I have a user that has a WordPress virus scanner installed and its also generating similar warnings. It would be great if we could find a way to get around this warning.

I noticed that there are other required includes in the plugin and those don't generate the warning. I'm not sure what is different about this particular include that is causing it.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 22, 2015

There is a reason why that's a warning not an error. And no, there is no other way to do that.

@GaryJones
Copy link
Member

Not a bug, but a reminder to investigate whether there's a way to include the file without triggering the Theme Check warning. i.e. use / not use parentheses etc.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 23, 2015

@GaryJones Interesting, it triggers on the parentheses ? Seems a bit overzealous on the theme check part.

@GaryJones
Copy link
Member

https:/Otto42/theme-check/blob/master/checks/include.php#L10 seems to be the relevant regex. Not checked it against what TGMPA uses, but there is a ? after the initial (, which should make it optional.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 23, 2015

@GaryJones Actually, looking at the regex, the warning is only triggered when the include/require uses parentheses - seems like a bug in ThemeCheck to me as includes/requires don't need parentheses.

All the same we could (ab)use the bug to get round the warning by just removing the parentheses in this line: https:/thomasgriffin/TGM-Plugin-Activation/blob/master/class-tgm-plugin-activation.php#L1060

FYI: The ? after a ( just indicates there is going to be a special command after it which applies to the group the ( started.

Compare:

$re = '`\(?`'; // optional literal parentheses - take note of the `\` before the parentheses
$re = '`(?:abc)`'; // `?:` means 'don't remember' - so this group is required, but won't be remembered as a sub-match
$re = '`(?#abc)`'; // `?#` turns whatever is in the parentheses into a regex comment, no matching done
$re = '`(?<!abc)`'; // `?<!` is a negative look-behind 

@GaryJones
Copy link
Member

Ah, sorry, I really rushed reading it. I can read / decipher regex, but only took a very quick glimpse this time.

I'm not sure if it ever made it to the WPCS, but I think there was agreement to use the parentheses format (maybe that was just for Genesis Framework). I do agree it's an oversight in Theme Check, but I'm also happy to abuse it (and I prefer without them anyway), for the sake of consistency of course... ;-)

@timnicholson
Copy link
Author

Thanks for looking into this, guys. As programmers, we know its not a real issue, but to users of our themes warning messages can cause concern. If there is a quick fix to get around the intricacies of the theme check plugin, then it may as well be done. The particular theme user that expressed concerns was using some WordPress anti-virus plugin, but I assumed it was similar to or perhaps directly used the theme check plugin as part of it.

I'm so grateful for this plugin as its the best way to have our open source themes free of bloat but still suggest (or require) other open source plugins to be able to provide similar features to all those "premium" theme sites out there.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 24, 2015

PRs opened for both develop as well as master branch. #262 / #263

@jrfnl jrfnl closed this as completed May 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants