[RFC] Get rid of all bypass of generate_text_for_* family

[brunoais]

In some places in the code, there's code that does just about the same as the generate_text_for_* family of functions.
For example:
includes/message_parser.php:+-1217
includes/functions_posting.php:+-1110
includes/functions_privmsgs.php:+-2024
includes/mcp/mcp_queue.php:+-141
amoung some others

The only difference here is that calling in those ways, it does not trigger the related events (core.modify_text_for_display_before, core.modify_text_for_storage_after, core.modify_text_for_edit_after, etc...) but if they start using that group of functions, they would, instead.

Why this RFC:
1st It's much, much easier and logical to place textFormatter in the those function group generate_text_for_* than placing in the bbcode class because, as far as I know, it's textFormatter that will do the text censoring and BBcode parsing.
2nd It makes to me a whole lot more sense to use those three generate_text_for_* instead of using the mix of all those other functions. If it wasn't for the event I wouldn't ask for it here.

So, there are 3 things that can be done.
1. Leave as is
2. Apply this RFC and add a parameter into the function such that the caller can choose if the events should be triggered or not (2nd preferred option)
3. Apply this RFC -> Replace all those manual text parsing with the call to the respective function.

[EXreaction]

All the bbcode parsing should really be done through the generate_text_for functions.

I would make a new ticket for each location that doesn't use them and create one PR for each location. If you want to do this I'll gladly review and merge them.

[brunoais]

Great.
Sure, I can do it.
It was part of the plans I made .

I'll create the generic ticket and I'll create a sub-task for each place I find that kind of thing.
Then I can call you @IRC so that we can review the most difficult cases.

Should I create a different PR for each case or a single PR will do?

[EXreaction]

Please do a different PR for each location, it'll make the process easier and faster for us.

Thanks!

[JoshyPHP]

Checking up on that one. The tracker entry is at https://tracker.phpbb.com/browse/PHPBB3-11635

From what I can tell, all of the rendering is now done through generate_text_for_display(), correct? I sifted through the sources and couldn't find any code that used the BBCode class directly. While doing so I noticed that bbcode instances were still created and left unused. I sent a PR that removes them: https://github.com/phpbb/phpbb/pull/3387

A number of files still use decode_message() instead of generate_text_for_edit(), and parse_message instead of generate_text_for_storage(). As far as my own PR is concerned it doesn't matter since it modifies decode_message()/parse_message and not the generate_text_for_* functions.

[brunoais]

I actually paused these solutions because I was told to, while a new BBCode parser was being thought.

[JoshyPHP]

Are you aware of any remaining locations that should use generate_text_for_display() but don't? I looked for it and I didn't find any so I assume there are none left.

[brunoais]

I checked... Some places I had identified were gone, other placed went to use the message parser directly or there's really no actual bypass towards that function due to how they work.
I wonder if I should just cancel all the PR that haven't been worked on yet...