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

feat(BuildCommand.php): add lifecycle hooks for prehtml and prepdf #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bmcminn
Copy link

@bmcminn bmcminn commented Nov 11, 2020

Adds user editable functions to ibis.php that allows users to modify
content via global event hook callbacks.

Fixes issue #13

@themsaid
Copy link
Owner

I think it's better if you don't have to define global functions. Just include a closure in the config file to be invoked within the build command. Also can you please include an example of how you're using both callbacks in a real scenario?

@bmcminn
Copy link
Author

bmcminn commented Nov 11, 2020

Yeah it's not ideal.

Reason I went this route was because the method signature for BuildCommand.php@buildHtml() doesn't have an argument for $config.

I think it makes more sense to make $config a property of the BuildCommand class and initialize it in the execute() method so we can just reference that instead of passing the $config value around from method to method. I'll get that worked in my next push.

As for an example:

function prehtml($markdown) {

    // remove frontmatter/comments/notes/etc
    $markdown = preg_replace("/-----/", '<hr>', $markdown); // convert horizontal rules first
    $markdown = preg_replace("/---[\s\S]*?---/", '', $markdown); // strip frontmatter content blocks
    $markdown = preg_replace("/\/\/[\s\S]+?\n/", '', $markdown); // strip inline C-style comments

    return $markdown;
}

The prepdf() method could be used in the same way for preprocessing the resulting HTML, like custom flags similar to the page [break] marker. This allows users to extend the render process as they see fit without the core library having to support all their edge case needs. If someone wanted to use a MathML library to convert their equation syntax then this gives them an opportunity to solve that themselves rather than needing Ibis to support it for them.

It also gives people a place to initialize content plugins at will, so my comment stripper could become a ibis-comment-stripper macro plugin that I initialize in my ibis.php file for portability. It's really up to the person what all they would want to do in there, but the core library doesn't have to do anything else to directly support that.

@bmcminn bmcminn force-pushed the master branch 7 times, most recently from a04cdf9 to 332ed57 Compare November 15, 2020 06:27
@bmcminn
Copy link
Author

bmcminn commented Nov 15, 2020

I think that's the final version of this feature.

To recap, I moved the $config variable from being a local method argument to being a private class field of BuildCommand class. I also updated each local $config reference to $this->_config and removed all $config arguments from the method signatures to avoid ambiguity.

So far I'm pretty happy with how this turned out.

Adds optional user defined functions to ibis.php that allows users to modify content

Fixes themsaid#13
@bmcminn bmcminn mentioned this pull request Nov 15, 2020
@bmcminn
Copy link
Author

bmcminn commented Nov 16, 2020

I think this feature is in pretty good shape for final review.

@bmcminn
Copy link
Author

bmcminn commented Nov 20, 2020

@themsaid any thoughts on accepting this PR?

@bmcminn
Copy link
Author

bmcminn commented Dec 1, 2020

@themsaid *bump

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.

3 participants