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

Add support for events #399

Closed
Arcesilas opened this issue Jan 9, 2020 · 4 comments · Fixed by #427
Closed

Add support for events #399

Arcesilas opened this issue Jan 9, 2020 · 4 comments · Fixed by #427
Labels
enhancement New functionality or behavior implemented Change has been implemented
Milestone

Comments

@Arcesilas
Copy link

Extensions would really take advantage of an events system, instead of walking through the AST one more time after the document has been parsed.

Events could be triggered, for example, when:

  • a block is about to be parsed
  • a block has been parsed
  • a block is about to be rendered
  • a block has been rendered
  • the document is about to be processed
  • the document has bee processed

Listening to events would allow to plug in the parsers and renderers directly while the document is beeing processed. In some cases, it prevents from going through the document multiple times.

@colinodell colinodell added enhancement New functionality or behavior feedback wanted We need your input! labels Jan 9, 2020
@colinodell
Copy link
Member

I really like the idea of leveraging events like this!

While I mull this over and try out a few different implementations, I'd love to hear any additional ideas or specific use cases that you or other members of the community have in mind (to ensure what we create best meets everyone's needs)

@Arcesilas
Copy link
Author

An example: create a table of content.

To create a ToC, you need to get some or all of the headings in a document. To build a ToC, currently, we need:

  • the list of the headings of the documents (or the headings of specific levels, e.g. 2-3 for a 2 level menu)
  • to render the headings with a link within, with a name attribute, that will be linked to from the ToC.

Currently, to achieve this, we have to add a renderer for headings, then parse the AST to get the headings list.
I've always considered that it's better to avoid looping multiple time over the same object. Events allow this. Also, accessing the AST requires to instanciate the environment then pass it to the converter.

With events, it would be much easier and would not require any additional renderer:

  1. instantiate a converter
  2. plug events listeners (parsed heading and rendering heading)
  3. that's all

The parsed heading listener would work like this:

  • receives the parsed block (League\CommonMark\Block\Element\Heading)
  • store the block depending on level
  • returns nothing

The rendering heading event:

  • receives the markdown and HTML
  • add the <a> tag around the finalStringContents
  • returns the HTML modified

The listeners may receive other data depending on what you think is relevant: maybe the environment.

The "block parsing listener" may alter the block element, or the environment (add a block parser or a renderer, maybe?). I don't know if it should return something.

The "renderering listener" should return the modified HTML.

I'm really not sure about what listeners should receive and return, I'm not familiar enough with the library. Maybe a block should offer an easy possibility to modify the content (it seems that currently the html element can be created with content, but it's not easy to modify it?).

@stale
Copy link

stale bot commented Mar 9, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Issue may be closed soon due to inactivity label Mar 9, 2020
@colinodell colinodell added the do not close Issue which won't close due to inactivity label Mar 9, 2020
@stale stale bot removed the stale Issue may be closed soon due to inactivity label Mar 9, 2020
@colinodell colinodell added this to the v1.4.0 milestone Apr 8, 2020
@colinodell colinodell linked a pull request Apr 13, 2020 that will close this issue
@colinodell colinodell modified the milestones: v1.4.0, v2.0 Apr 17, 2020
@colinodell
Copy link
Member

2.0 will have three key events dispatched at critical points during the parsing and rendering process. We've also added new features like ToC and Front Matter using both events and other built-in extension points.

I don't see the need to add any more events at this time, but I am definitely open to adding more in the future, and it should be easy to do this without breaking BC :) I'm therefore going to mark this issue as resolved. I remain open to any requests to add specific new events, I only ask that they are accompanied by a specific use case to help us confirm that they're the best way to accomplish the needed task.

Thank you very much for your suggestion!

@colinodell colinodell added implemented Change has been implemented and removed feedback wanted We need your input! do not close Issue which won't close due to inactivity labels Jun 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality or behavior implemented Change has been implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants