PHPBB3-13803 / PHPBB3-13891 - Reparsing text

Discuss requests for comments/changes posted in the Issue Tracker for the development of phpBB. Upcoming releases are 3.2/Rhea and 3.3.
Post Reply
User avatar
JoshyPHP
Registered User
Posts: 348
Joined: Fri Jul 08, 2011 9:43 pm

Re: PHPBB3-13803 / PHPBB3-13891 - Reparsing text

Post by JoshyPHP » Sat Dec 17, 2016 11:44 am

I wouldn't recommend setting it to the plugin's name. As far as I know, only 2 values are ever used for that parameter outside of the reparser: "post" and "sig". Keeping it that way would be the safe option.

User avatar
rxu
Registered User
Posts: 124
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: PHPBB3-13803 / PHPBB3-13891 - Reparsing text

Post by rxu » Sat Dec 17, 2016 11:50 am

Well yes, but reparser handles more places (posts, PMs, polls, forum rules, group descriptions, signatures), so having just 2 options is kind of not enough.
Image

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

Re: PHPBB3-13803 / PHPBB3-13891 - Reparsing text

Post by JoshyPHP » Sat Dec 17, 2016 12:09 pm

If you use the plugin's name as mode, be aware than the plugin that reparses users signatures is named user_signature whereas the mode used when editing a signature the conventional way is sig. Same for post_text and post.

User avatar
rxu
Registered User
Posts: 124
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: PHPBB3-13803 / PHPBB3-13891 - Reparsing text

Post by rxu » Sat Dec 17, 2016 2:00 pm

What about that (within the reparser's base.php)

Code: Select all

	/**
	* Gets the current reparser plugin class name
	*/
	public function get_name()
	{
		$reflection = new \ReflectionClass($this);
		return $reflection->getShortName();
	}
and then

Code: Select all

		generate_text_for_storage(
			$text,
			...
			$unparsed['enable_url_bbcode'],
			'reparse.' . $this->get_name()
		);
Image

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

Re: PHPBB3-13803 / PHPBB3-13891 - Reparsing text

Post by JoshyPHP » Sat Dec 17, 2016 2:07 pm

The issue is the same: I don't see any benefit to introducing new modes. As far as I'm aware there are only 3 values ever used for $mode in phpBB 3.2: "post", "sig" and since 3.2, "reparse". I think it's safer to ditch "reparse" and just keep the two known ones.

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

Re: PHPBB3-13803 / PHPBB3-13891 - Reparsing text

Post by VSE » Sun Dec 18, 2016 4:56 pm

JoshyPHP wrote:
Sat Dec 17, 2016 11:33 am
I think that the correct way to do it would be a big overhaul of message_parser and the generate_text_for_* functions into a unified API that supports some sort of contextual configuration that would allow the text to be treated differently depending on whether it's from a post, a signature, etc...

For a more immediate solution, reparser_interface could be extended to some sort of get_parsing_mode() function, the return value of which would be passed to generate_text_for_storage(). Currently, it is hardcoded to "reparse", which, in hindsight, is not a good value. phpbb\textreparser\base::get_parsing_mode() could return "post" by default and the user_signature plugin would return the appropriate value instead. As I recall, that value is already present in the relevant events.
I made a PR here to set names for reparsers:
https://github.com/phpbb/phpbb/pull/4585

I'm not so sure setting 'reparse' to 'post' is a good idea for the reparsers by default. Because when you use post, it enforces settings like max/min characters allowed. If users have changed that setting over the years, reparsing could fail on posts that are too big or too small. In fact it seems the post mode is only used to enforce max/min characters, smilies, URLs, etc.

In the PR, the "short" name of the reparser is used instead of just 'reparse'. Like 'reparse' it's just not used for anything other than to allow the reparsers to succeed without worrying about posting limits, but by using the reparsers name it is a bit specific which is nice if the core or an extension wants to target a specific reparser action.
Has an irascible disposition.

Post Reply