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

Make it easier to add multiple extensions #686

Open
Atulin opened this issue Dec 7, 2018 · 5 comments
Open

Make it easier to add multiple extensions #686

Atulin opened this issue Dec 7, 2018 · 5 comments
Projects
Milestone

Comments

@Atulin
Copy link

Atulin commented Dec 7, 2018

Currently, the only way to register multiple extensions would be

class Foo extends ParsedownExtra {}
class Bar extends Foo {}
class Baz extends Bar {}
class Foobar extends Baz {}

ending in

$parsedown = new Foobar();

IMHO it's rather counterintuitive and counterproductive, especially compared to other solutions – like what Twig does, for example:

$twig = new Twig_Environment($loader);
$twig->addExtension(new Twig_Extensions_Extension_Text());
$twig->addFunction(new \Twig_SimpleFunction('asset', function ($asset) {
    return sprintf('/public/%s', ltrim($asset, '/'));
}));

I propose we should at least discuss the possibility of using a similar approach, i.e.

$pd = new Parsedown();
$pd->extend(new ParsedownExtra());
$pd->extend(new Foo());
$pd->extend(new Bar());
$pd->extend(new Baz());
$pd->extend(new Foobar());
@aidantwoods
Copy link
Collaborator

See also: #534

This is definitely something that I want to address. I think I'd be looking at more of an interface based approach to "extensions" going forward as opposed to the current situation—these could be used in a way similar to how you've demonstrated, and we wouldn't need extensions to depend on each other in order to work within the same Parsedown instance.
Related to this problem is the reason behind not releasing the improvements in the 1.8.0-beta* releases as stable: even though there are no breaking changes to the public API, there are breaking changes on the protected API level that do impact extensions. For this reason I think it is best that the encouraged way of developing "extensions" isn't writing literal class extensions, which don't have a clear API boundary about behaviour they can rely on across updates.

@aidantwoods aidantwoods added this to the 2.0.0 milestone Dec 11, 2018
@Atulin
Copy link
Author

Atulin commented Dec 11, 2018

Glad to hear it's on the roadmap 👌

@aidantwoods
Copy link
Collaborator

Once it's finished, this should be solved by: #685

@glensc
Copy link

glensc commented Mar 10, 2019

you can use decorator pattern until now:

class Extension1 {
   public function __construct($parent) {
      $this->parent = $parent;
   }

   public function bla($arg) {
     $res = $this->parent->bla($arg);
     $res = dosomething($res);
     return $res;
   }

   // proxy everything unknown to parent
   public function __call($method, $arguments) {
      return call_user_func_array([$this->parent, $method], $arguments);
   }
}
$parsedown = new Parsedown();
$ext1 = new Extension1($parsedown);
$ext2 = new Extension2($ext1);

this way the Extension1, Extension2 code does not have to be modified if you want to change the injected parser.

this is not ideal, you still need to modify extension1 and extension2 code at least once.

@aidantwoods aidantwoods added this to To do in 2.0.0 via automation Apr 8, 2019
@aidantwoods aidantwoods moved this from To do to To review in 2.0.0 Apr 8, 2019
@aidantwoods aidantwoods moved this from To review to Review in progress in 2.0.0 Apr 8, 2019
@aidantwoods aidantwoods moved this from To review to In progress in 2.0.0 Apr 24, 2020
@aidantwoods aidantwoods moved this from In progress to Done in 2.0.0 Oct 11, 2021
@aidantwoods
Copy link
Collaborator

It's not released yet, but I've explained a little how I think this will work going forward over in: #708 (comment). Feedback welcome!

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

No branches or pull requests

3 participants