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

Provides plugin INSTANCES from Grav instance #2277

Closed
wants to merge 1 commit into from

Conversation

drzraf
Copy link
Contributor

@drzraf drzraf commented Nov 29, 2018

Don't consider users babies, save them digging stacktraces to find the data they need : Just provide a way to access currently initialized plugins INSTANCES.

Sample use-case:

  • I want to modify a form in PHP.
  • The hooks provided by form (onFormPageHeaderProcessed) does not make it because I want to know whether or not to add assets according to the form.
    (There would have many other legitimate use-case for accessing instance data)

Without this patch, form plugin forms are unavailable. With it, there are just here:

public function onTwigSiteVariables(Event $event) {
        dump(Grav::instance()['plugins']->item('Grav\Plugin\FormPlugin')->getForm());
}

Just provide (a way) to access current plugin INSTANCES..
@mahagr
Copy link
Member

mahagr commented Dec 1, 2018

To be honest, I'm strongly against allowing users to access plugins directly. Instead, the plugins should be implemented so that they provide services that you can use.

@drzraf
Copy link
Contributor Author

drzraf commented Dec 1, 2018

Nor cleanness nor dirtiness are objectives by themselves. I understand anyone would prefer to have a nice and clean API with fences all around in order to protect users (actually: "developers") from shooting themselves in the foot, or to rely on methods that may disappear later, ...

I also agree that granting plugin instance access is a necessary but not a sufficient condition.
See for example: getgrav/grav-plugin-form#307

But a very important characteristic of a good software is to be able to be tweaked so that it fits our needs and not only in the common use-case that initial developers thought about.
It's frustrating to know the data we want to use is here but was just made voluntarily unavailable, behind a fence, just for Cleanness purpose. Some Drupal users may know about this "hostage syndrome" :)

When such a real-world case happens to you, cleanness is worth nothing in comparison with resolving the problem. I'm sorry if this sound manichean, but that's a kind of Drupal vs WordPress issue.

Please, just let me use my (plugin) data during the processing pipeline :)

@mahagr
Copy link
Member

mahagr commented Dec 1, 2018

Yes, my main reason not allowing you to do that is that it's not considered to be part of API and your code can break on any release, including bugfix (+0.0.1) releases. The trend has been to make everything private or final so that you cannot even extend the classes anymore -- for the same reasons. This includes twig, Symfony and about every library I'm using.

I've been customizing forms by page content or URL and I've never needed to access the plugin directly. Maybe there's already a way to do this? Let me comment to the another issue.

@drzraf
Copy link
Contributor Author

drzraf commented Feb 19, 2019

The end-result is that instead of living with code-that-can-break-on-update during months, I need to live with composer-patches + code-that-can-break-on-update during months.

@mahagr
Copy link
Member

mahagr commented Feb 10, 2021

@drzraf You should already be able to get plugins by $plugins['Grav\Plugin\FormPlugin'].

@drzraf
Copy link
Contributor Author

drzraf commented Feb 10, 2021

The last time I tried in 1.6 (years ago), I was only getting a stripped-down version of the object: The wrapping interface kept me from accessing particular plugins' methods and attributes. I'd be happy to know this limitation been removed in 1.7.

mahagr added a commit that referenced this pull request Feb 11, 2021
@mahagr
Copy link
Member

mahagr commented Feb 11, 2021

Added methods to get plugins by their name, not by class (as it already exists).

BTW: there's no stripped-down version of the plugins.. They have always been the full objects.

@mahagr mahagr closed this Feb 11, 2021
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