TGMPA / TGM-Plugin-Activation

TGM Plugin Activation is a PHP library that allows you to easily require or recommend plugins for your WordPress themes (and plugins). It allows your users to install, update and even automatically activate plugins in singular or bulk fashion using native WordPress classes, functions and interfaces. You can reference bundled plugins, plugins from the WordPress Plugin Repository or even plugins hosted elsewhere on the internet.
http://tgmpluginactivation.com/
GNU General Public License v2.0
1.76k stars 431 forks source link

Fix the example 'source' path to work with child themes #678

Open kevinvess opened 7 years ago

kevinvess commented 7 years ago

By using the get_template_directory() function, it retrieves the absolute path to the directory of the current theme; and if a child theme is being used, the absolute path to the parent theme directory will be returned.

I think it makes more sense to use this method in the example script in case theme developers are just copying this without testing if it works for child themes. I would imagine this gets used for more parent themes than child themes anyway.

jrfnl commented 7 years ago

Hi @kevinvess Thanks for your PR. I get where you are coming from, and this is handled by the Custom TGMPA Generator which is the recommended way to download TGMPA. If you download a copy through the Custom Generator, you will see that the path in the source parameter has been adjusted automatically based on whether you choose parent theme / child theme or plugin.

jrfnl commented 7 years ago

@kevinvess Did my response clarify things ? can I close the PR ?

kevinvess commented 7 years ago

Thanks for the response; I do see the Custom Generator does handle applying the correct function depending on how the user intends to use this library.

However, I still think my PR is worth merging. Its a small change that will help ensure that anyone who copies this library from GitHub instead of using your Custom Generator will not run into issues when adding a child-theme later – as is what happened to me.

Do what you will with this, its your code repository.

GaryJones commented 7 years ago

@jrfnl From your previous stats, do you have any indication about whether TGMPA is included more with parent or child themes?

jrfnl commented 7 years ago

@GaryJones I don't currently have those kind of stats, just theme vs plugin and # per version of TGMPA.

There is a sniff proposal is the TRT fork of the WordPress Coding Standards which looks for TGMPA and does some checks. I could add some metrics to that sniff and run it over the theme repo. That should give some indication, though only of the themes published on WP.org - which isn't really all that relevant as themes published on WP.org have to use a version generated using the Custom TGMPA Generator.

Running the sniff over the theme repo will probably take a couple of hours (aside from having to sync the theme repo again first), so it may be a little while before I could have figures.

GaryJones commented 7 years ago

I consider this to be minor, so if it doesn't get addressed until a later release, so be it.

jrfnl commented 7 years ago

Ok, I've ran the metrics for the whole theme repository and posted the results in the sniff thread: https://github.com/WPTRT/WordPress-Coding-Standards/pull/132#issuecomment-320471524

I have to reiterate that these results are not all that relevant as they are based on the WP.org theme repo where themes are required to use the Custom Generator anyway.

While the results lean towards TGMPA being included more with parent themes than child themes (not surprisingly), the results are not conclusive as the child/parent theme status is unknown of 43% of the themes using TGMPA (= themes using outdated versions and themes using a TGMPA version not generated using the Custom TGMPA Generator, mostly because they were published before it was available).

Thinking this over, these are the potential usage combinations I can think of and whether either function would work out of the box, where the third option is probably least in use:

Theme active Prepackaged plugin provided by Config called from get_template_directory() get_stylesheet_directory()
parent parent parent :white_check_mark: :white_check_mark:
child parent parent :white_check_mark: :x:
child parent child :white_check_mark: :x:
child child child :x: :white_check_mark:

If my thinking here is correct (and please verify for yourselves), then based on this table, there is sufficient justification for making this change.

If it would be so decided, however, this PR will need a sister-PR to adjust the logic in the Custom TGMPA Generator before this can be merged.

GaryJones commented 7 years ago

Thank you for the investigation and write-up.

there is sufficient justification for making this change.

If it would be so decided, however, this PR will need a sister-PR to adjust the logic in the Custom TGMPA Generator before this can be merged.

Let's do it :-)

jrfnl commented 7 years ago

OK, so open actions: