Skip to content

Use correct directory path#469

Closed
emiluzelac wants to merge 1 commit intoTGMPA:developfrom
emiluzelac:patch-1
Closed

Use correct directory path#469
emiluzelac wants to merge 1 commit intoTGMPA:developfrom
emiluzelac:patch-1

Conversation

@emiluzelac
Copy link
Copy Markdown

get_template_directory() should be used in both cases.

  • get_template_directory(): Returns the absolute template directory path.
  • get_stylesheet_directory(): Returns the absolute stylesheet directory path.

Maybe you could recommend https://wordpress.org/plugins/example/ instead of one that bundles.

`get_template_directory()` should be used in both cases. 

- get_template_directory(): Returns the absolute template directory path.
- get_stylesheet_directory(): Returns the absolute stylesheet directory path.
@GaryJones
Copy link
Copy Markdown
Member

Thanks Emil.

There are several things wrong here:

  • You're assuming that TGMPA is being used within a theme. For plugins that use TGMPA, get_template_directory() . '/class-tgm-plugin-activation.php' won't be valid. Using dirname() is plugin/theme agnostic.
  • You're assuming that class-tgm-plugin-activation.php is within the top-level of a theme. It's far more likely, and therefore a better default, to find a TGMPA setup file within the same directory as the class, regardless of how many directories deep that may be. dirname( __FILE__ ) is therefore a better default.
  • Even if TGMPA was only for themes, child themes may want to include it when parent themes don't have so get_stylesheet_directory() would be more accurate anyway, which is why we use it in your second change. This bit does assume it's being used in a theme, but it's also a bit we expect to be edited, whereas the requiring of the class may not.

We already include examples from the WPORG repo (BuddyPress), along with bundled plugin, external zip, GitHub repo and a special case.

@emiluzelac
Copy link
Copy Markdown
Author

My bad, I wasn't thinking about the plugins at all :(

As far as dirname( __FILE__ ) maybe you could add a note somewhere that this would need to be changed accordingly if included with themes that is. This is a required change for .org and ThemeForest as well.

@emiluzelac emiluzelac closed this Aug 28, 2015
@GaryJones
Copy link
Copy Markdown
Member

How about you do a PR which has commented-out versions suitable for plugin and theme, replacing what's currently there?

e.g.

// Uncomment ONE of the following typical paths and amend as necessary.
// Theme:
// require get_stylesheet_path() . '/path/to/class-tgm-plugin-activation.php';
// Plugin:
// require {whatever the plugins_dir_path() thing is that I'm too lazy to look up right now  :-) };

@emiluzelac
Copy link
Copy Markdown
Author

I'll try, time isn't on my side lately :)

On Fri, Aug 28, 2015 at 2:52 PM, Gary Jones notifications@github.com
wrote:

How about you do a PR which has commented-out versions suitable for plugin
and theme, replacing what's currently there?

e.g.

// Uncomment ONE of the following typical paths and amend as necessary.// Theme:// require get_stylesheet_path() . '/path/to/class-tgm-plugin-activation.php';// Plugin:// require {whatever the plugins_dir_path() thing is that I'm too lazy to look up right now :-) };


Reply to this email directly or view it on GitHub
#469 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants