Add strip_bbcode() method to String

[ztc0611]

Replaces #65384.

Adds a function to strip bbcode tags out of strings.

Example Code:

var s = "[color=#ffffff]Hello![/color]
print(s.strip_bbcode())

Result: Hello

Closes godotengine/godot-proposals#5056

[markdibarry]

This is a great idea. Haven't looked in awhile, but if the implementation is the same, would it make more sense to add this to the TextServer and have both RTL and string call the same method? Or do they need to be separate methods?
@bruvzg Maybe you could weigh in?

[ztc0611]

would it make more sense to add this to the TextServer and have both RTL and string call the same method?

I'm still not a big expert on Godot's code. I wasn't able to find the function that does that to RTLs already (to convert them to the text property), but if that is considered to be a better solution it's fine by me.

[Spartan322]

would it make more sense to add this to the TextServer and have both RTL and string call the same method? Or do they need to be separate methods?

I don't see why you would call it in the RTL given that if you're using BBCode it has to process the text and if not then you want it to be a raw string including the BBCode tags.

[ztc0611]

I don't see why you would call it in the RTL given that if you're using BBCode it has to process the text and if not then you want it to be a raw string including the BBCode tags.

I think they meant make the RTL and String use the same function for striping it, because the RTL converts bbcode_text to raw text in the editor. I didn't tap into that exact functionality though.

[markdibarry]

After some digging, it looks like RTL is in fact not caching but stripping it every time you call get_parsed_text(), but it looks like it's doing something specific, so I'm going to say we should keep them separate unless otherwise needed.

[dalexeev]

Actually, there is a way to escape BBCode. Maybe we should add escape_bbcode instead of / together with strip_bbcode?

[ztc0611]

So it needs a check to make it convert [lb] and [rb] to brackets? I can start on that soon.

[markdibarry]

The other pretty important issue with this is that it doesn't check anywhere if it's valid BBCode or not. This is important, since RichTextLabel doesn't strip BBCode that doesn't match a tag.

[ztc0611]

@markdibarry any idea where the script that checks for valid bbcode is?

[dalexeev]

RichTextLabel doesn't strip BBCode that doesn't match a tag

Given that custom BBCodes can be added, I think it's correct to only consider the syntax (strip all tags, including non-existent and non-matching ones) and point this out in the documentation.

[Spartan322]

The other pretty important issue with this is that it doesn't check anywhere if it's valid BBCode or not. This is important, since RichTextLabel doesn't strip BBCode that doesn't match a tag.

The only simple independent way I can think of this being implemented is by having a list of valid tag names in an array and "skipping" past anything outside that array, honestly doesn't look like that be difficult, (perhaps that can be considered, if so however it should be under a different name so it'll handle the default tags by distinctly and by default like strip_bbcode_tags(handle_defaults : bool = true, extra_tags : Array[string] = [])) but if the function gets that complex then doesn't trying to better comply with BBCode enter the fray adding undue complexity?

[ztc0611]

If custom BBCode tags can be added, and RichTextLabel can access this, then surely this function could just automatically access whatever that store is, right? That seems like it would be the most elegant solution.

[Spartan322]

If custom BBCode tags can be added, and RichTextLabel can access this, then surely this function could just automatically access whatever that store is, right? That seems like it would be the most elegant solution.

That's the clunkiest solution actually, as you would be required to pass the RTL every time you made a call and makes the function heavily dependent upon the RTL. (meaning as well if you don't use the RTL now you're polluting the dependency of the class just for that function, which iirc is actually prohibited as new additions anyway as core can't be dependent upon scene files) And that aside its unnecessary since all the information this function needs to remove known tags is the name of the tags, as in all you need is strings, you don't need anything else in the RTL and don't need to keep a reference to an RTL in order to use it.

[ztc0611]

That's the clunkiest solution actually, as you would be required to pass the RTL every time you made a call and makes the function heavily dependent upon the RTL.

Considering the primary use case here is for RTLs, maybe this should be a function attached to RTLs? Like $RichTextLabel.strip_bbcode(string) which just returns the stripped text as the RTL would display it, and doesn't affect the RTL's actual bbcode_text or text? It not returning an identical string to what the RTL will end up displaying feels like it could make this function have limited use for many.

[dalexeev]

I guess the main purpose of strip_bbcode is to safely add text without markup (although there is add_text and [lb], [rb] tags). In order to get the current visible text there is get_parsed_text. strip_bbcode respecting the current set of tags is useful if you need to know the future visible text, but I think this is a rare use case.

[markdibarry]

useful if you need to know the future visible text, but I think this is a rare use case

Funny enough, that's the exact use case I have. When working with dialog, knowing the indices of rendered characters can be crucial. With any custom text syntax, your two options are to manually keep a list of valid bbcode tags up to date, or:

1. Set the text, so the text can be parsed
2. call get_parsed_text()
3. Use the full text and parsed text together to determine the rendered text.
4. Parse the text for custom syntax.
5. set the text again.

Can be a little over-the-top.

[ztc0611]

When working with dialog, knowing the indices of rendered characters can be crucial.

This is my exact use case as well, but I don't use custom bbcode. Wasn't sure if it was an odd edge case or not. I use it to make the text make sounds only for precise durations, and pause when the text reaches a pause.

[markdibarry]

Sounds rad. Keep me posted with your project's progress after this!

[dalexeev]

When working with dialog, knowing the indices of rendered characters can be crucial.

I prefer not to rely on indexes, but to use invisible control characters.

According to your description, it should not be String.strip_bbcode, but RichTextLabel.get_parsed_text_from_string or something like that. strip_* means that some character sequences will be removed (like strip_escapes or lstrip) and not replaced (like c_escape, c_unescape, etc.).

We also need to remember that rtl.text += s and rtl.append_text(s) give different results for unclosed tags. And the proposed behavior may not be suitable in one of the cases.

[dalexeev]

I guess strip_bbcode should just remove all BBCodes without parsing them (even [code]). That is:

"[lb]text[rb]".strip_bbcode() # "text", not "[text]"

Otherwise, after strip_bbcode, we can get a string containing the BBCode, which even sounds wrong:

"[lb]b[rb]text[lb]/b[rb]".strip_bbcode() # "[b]text[/b]"

As I wrote above, if the functionality is needed, then it should not be the String.strip_bbcode method, but RichTextLabel.get_parsed_text_from_string or something similar.

[ztc0611]

But what if they want brackets in the text they're typing for some reason? I don't see why example 2 would ever come up outside of somebody trying to intentionally "break" it.