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

Allow users to override plugin handler priorities #2165

Merged
merged 2 commits into from
Aug 12, 2019
Merged

Allow users to override plugin handler priorities #2165

merged 2 commits into from
Aug 12, 2019

Conversation

Perlkonig
Copy link
Contributor

@Perlkonig Perlkonig commented Aug 31, 2018

Not sure how pressing this is, but I've seen it once or twice on the forums, and it seems easy enough to fix.

Most plugin authors leave the priority of their listeners at 0 unless there is some internal reason to tweak them. But no plugin author can predict what sorts of interactions may exist with all other plugins. If a user discovers that they need certain plugins to run before others, the only current way to change that is by modifying the priorities in the plugin's .php file. Which will get clobbered every time the plugin is updated.

This PR allows users to create a user/config/priorities.yaml file that may contain the following data:

pluginName:
  eventName:    #for example, "onPagesInitialized"
    listenerName: [integer]

The PR replaces the ternary if/else statement in Grav\Common\Plugin::enable with a new function getPriority that now has three possibilities instead of two:

  1. If a value is provided in priorities.yaml, it is returned.
  2. Otherwise it uses the value from the plugin's .php file, if provided.
  3. Otherwise it returns 0.

This is advanced usage that still requires users to do some digging to find the listeners that need adjusting. This PR is backwards compatible unless there's some other use for user/config/priorities.yaml that I'm not aware of. It does appear to work with plugins with hyphens in their names.

I will draft up a PR to grav-learn to match this approach. If implemented, I can see someone developing an admin plugin to make it really easy to make these adjustments.

This is only one approach. There may be very good reasons why this approach doesn't work. I'm open to any feedback. I don't know how to test the effect my changes have on performance. I'm open to instruction.

@mahagr
Copy link
Member

mahagr commented Aug 31, 2018

Interesting thought; I've been thinking of ways of improving plugin events and this is certainly one way to improve it.

@Perlkonig
Copy link
Contributor Author

Just checking in on old PRs.

@rhukster rhukster merged commit d2833a1 into getgrav:develop Aug 12, 2019
@rhukster
Copy link
Member

Thanks! Definitely, need to get this documented...

@Perlkonig
Copy link
Contributor Author

Thanks! PR to learn already submitted.

@rhukster
Copy link
Member

I didn't see the Learn PR, do you have a URL?

@Perlkonig
Copy link
Contributor Author

I referenced up top. Here it is again.

getgrav/grav-learn#643

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

Successfully merging this pull request may close these issues.

3 participants