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.
[RFC] Get rid of all bypass of generate_text_for_* family
- EXreaction
- Registered User
- Posts: 1555
- Joined: Sat Sep 10, 2005 2:15 am
Re: [RFC] Get rid of all bypass of generate_text_for_* family
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.
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.
Re: [RFC] Get rid of all bypass of generate_text_for_* family
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?
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
- Registered User
- Posts: 1555
- Joined: Sat Sep 10, 2005 2:15 am
Re: [RFC] Get rid of all bypass of generate_text_for_* family
Please do a different PR for each location, it'll make the process easier and faster for us.
Thanks!
Thanks!
Re: [RFC] Get rid of all bypass of generate_text_for_* family
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
A number of files still use
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/3387A 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.Re: [RFC] Get rid of all bypass of generate_text_for_* family
I actually paused these solutions because I was told to, while a new BBCode parser was being thought.
Re: [RFC] Get rid of all bypass of generate_text_for_* family
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.Re: [RFC] Get rid of all bypass of generate_text_for_* family
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...
I wonder if I should just cancel all the PR that haven't been worked on yet...