[RFC] Integrate s9e\TextFormatter

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.2/Rhea branch. Everything listed in this forum will be available in phpBB 3.2.
Post Reply
Nicofuma
3.2 Release Manager
3.2 Release Manager
Posts: 297
Joined: Sun Apr 13, 2014 1:40 am
Location: Paris

Re: [RFC] Integrate s9e\TextFormatter

Post by Nicofuma » Mon Mar 23, 2015 8:05 am

What is the issue with this extension?
Member of the phpBB Development-Team
No Support via PM

Cin
Registered User
Posts: 20
Joined: Mon Mar 23, 2015 11:33 am

Re: [RFC] Integrate s9e\TextFormatter

Post by Cin » Mon Mar 23, 2015 11:37 am

Nicofuma wrote:Last call, does someone have anything else to ask or anything to object about the features, do it now.
Well, I'd like to see that when you quote someone, the textformatter automatically links the username to the post in question. This would make it ALOT easier for people to read the original post if snippets of it are quoted and you want to read the full post. Specially on busy fora this is needed because otherwise you manually have to go through the previous pages to search for it.

Or at least make an option available in the backend...

User avatar
JoshyPHP
Registered User
Posts: 346
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Mon Mar 23, 2015 11:39 am

There's at least one event in the 3.1 routines that does not exist or trigger in 3.2. For instance, 3.1 loads custom BBCodes from the DB on every page they're displayed, which triggers core.modify_bbcode_init. In 3.2 they're loaded once during configuration and never again until the data changes / the cache is invalidated. ABBC relies on core.modify_bbcode_init to toggle BBCodes depending on the user's group memberships.

In 3.2, one way to toggle BBCodes is through the s9e\TextFormatter parser. You can get it with $container->get('text_formatter.parser')->get_parser(). One could do that in a core.modify_text_for_storage_before event, but it's a bit wonky. Not only it would have to be wrapped in a if ($container->has('text_formatter.parser')) test for backward compatibility, it also relies on the assumption that this text formatter is the one that will be used to parse the text. It makes compatibility—both backward and forward—inconvenient. That's why there should be an event that specifically applies to this text formatter.
Image Image

Nicofuma
3.2 Release Manager
3.2 Release Manager
Posts: 297
Joined: Sun Apr 13, 2014 1:40 am
Location: Paris

Re: [RFC] Integrate s9e\TextFormatter

Post by Nicofuma » Tue Mar 24, 2015 7:46 pm

I agree, there should be both parser dependent events (for fine tuning) and general events. Do go ahead and add these events please.
Member of the phpBB Development-Team
No Support via PM

User avatar
JoshyPHP
Registered User
Posts: 346
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Tue Mar 24, 2015 10:27 pm

I'll do that ASAP, probably tomorrow. I'm in contact VSE about extensions and he's telling me that tests hang on his machine. From what I gather it's a problem with his version of PCRE. (8.02) With brunoais suffering from a similar problem before upgrading his version of PHP I suspect it's due to mismatched versions of PHP and PCRE on MacOS. I'll update the library some time this week to circumnavigate this issue. I assume you'll want to wait for that to be fixed before merging this branch.
Image Image

Nicofuma
3.2 Release Manager
3.2 Release Manager
Posts: 297
Joined: Sun Apr 13, 2014 1:40 am
Location: Paris

Re: [RFC] Integrate s9e\TextFormatter

Post by Nicofuma » Tue Mar 24, 2015 10:43 pm

By the way, how will be impacted this event? https://tracker.phpbb.com/browse/PHPBB3-13648
Member of the phpBB Development-Team
No Support via PM

User avatar
JoshyPHP
Registered User
Posts: 346
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Tue Mar 24, 2015 11:10 pm

core.modify_bbcode_init is not trigger by a new message, so it wouldn't apply. Any extension that is coded too specifically against the current parser needs to be updated for the new architecture. Extensions that create custom BBCodes are fine, so are extensions that modify posts before they're parsed (core.modify_text_for_storage_before) or after they're rendered (core.modify_text_for_display_after) but anything that is too tightly coupled with the current parser may work differently or not work at all without changes.

Similarly, s9e\TextFormatter supports custom validators (only on attributes/BBCode parameters though) but unless someone proposes an API to be implemented in a text_formatter service, an extension coded against s9e\TextFormatter's validators/filters will only work with that library.
Image Image

Nicofuma
3.2 Release Manager
3.2 Release Manager
Posts: 297
Joined: Sun Apr 13, 2014 1:40 am
Location: Paris

Re: [RFC] Integrate s9e\TextFormatter

Post by Nicofuma » Tue Mar 24, 2015 11:25 pm

ok, so add the events necessary to do that for the library (anyway these events needs to be added) and later we could work on a more powerful text_formatter API
Member of the phpBB Development-Team
No Support via PM

User avatar
JoshyPHP
Registered User
Posts: 346
Joined: Fri Jul 08, 2011 9:43 pm

Re: [RFC] Integrate s9e\TextFormatter

Post by JoshyPHP » Thu Mar 26, 2015 6:29 pm

The following events are now in the PR:
  • core.text_formatter_s9e_configure_before - triggers before the s9e\TextFormatter configurator is configured with the default settings.
  • core.text_formatter_s9e_configure_after - triggers after the default settings. Both events can be used to configure anything related to BBCodes, censored words or smilies.
  • core.text_formatter_s9e_parser_setup - triggers when the text_formatter.parser service is loaded. Can be used to toggle features, set vars or plug custom parsers. Anything that only has to be executed once.
  • core.text_formatter_s9e_parse_before - triggers before a text is parsed.
  • core.text_formatter_s9e_parse_after - triggers after a text is parsed. The parsed text is XML so I hope people won't misuse the event.
  • core.text_formatter_s9e_renderer_setup - triggers when the text_formatter.renderer service is loaded. Can be used to set template variables.
  • core.text_formatter_s9e_render_before - triggers before a parsed text is rendered. The parsed text is XML, same concerns apply.
  • core.text_formatter_s9e_render_after - triggers after a text is rendered to HTML. I think it overlaps with core.modify_text_for_display_after. I haven't had time to measure the overhead of triggering events but if that's a concern, I think those two events could be removed.
Following up on VSE's PCRE woes, I've made sure that the library is tested on PHP 5.3.3 (PCRE 8.02 19-Mar-2010) on Travis. The same revision that crashed on VSE's Mac ran without a hitch on Travis. I'm not surprised, because the tests suite used to be run on 5.3.3 before I accidentally changed it to 5.3 a few months ago. Regardless, the code paths that could have been responsible for the crashes are now disabled on PCRE < 8.13 (16-Aug-2011.) This does not impact any functionality.
Image Image

User avatar
VSE
Extension Customisations
Extension Customisations
Posts: 670
Joined: Mon Mar 08, 2010 9:18 am

Re: [RFC] Integrate s9e\TextFormatter

Post by VSE » Thu Mar 26, 2015 10:41 pm

I can confirm that JoshyPHP has fixed PCRE issues and is doing an otherwise awesome job with this new framework for bbcodes!!
Has an irascible disposition.

Post Reply