Skip to content

Add core.strip_escapes() - #16485

Merged
sfan5 merged 6 commits into
luanti-org:masterfrom
sfan5:strip
Sep 14, 2025
Merged

Add core.strip_escapes()#16485
sfan5 merged 6 commits into
luanti-org:masterfrom
sfan5:strip

Conversation

@sfan5

@sfan5 sfan5 commented Sep 8, 2025

Copy link
Copy Markdown
Member

To do

This PR is Ready for Review.

@sfan5 sfan5 added @ Script API Feature ✨ PRs that add or enhance a feature labels Sep 8, 2025
Comment thread builtin/common/misc_helpers.lua Outdated
@rubenwardy

Copy link
Copy Markdown
Member

What's the use case here? If it's "I want to remove all color and translation tokens" then the user should do server-side translation into English and strip colours.

Just stripping the translation escape codes is quite questionable and unlikely to be what the user wants. It doesn't handle parameters in text

@Zughy

Zughy commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

@rubenwardy I'd use it to simplify this function, where I can't be sure whether the translated error message is sent to a player (translated) or in console (not translated)

https://gitlab.com/zughy-friends-minetest/arena_lib/-/blob/master/src/utils/utils.lua?ref_type=heads#L28

Alternatively, I could call NS(..) on every string when I call the function (i.e. arena_lib.print_error(foo, NS("My string"))) to then translate it only when it's sent to a player, but the other way feels cleaner

@sfan5

sfan5 commented Sep 9, 2025

Copy link
Copy Markdown
Member Author

What's the use case here? If it's "I want to remove all color and translation tokens" then the user should do server-side translation into English and strip colours.

Then we add font escapes next and the mod code is insufficient again. Small inconvenience sure, but being guaranteed to return plain text is the entire point.

Just stripping the translation escape codes is quite questionable and unlikely to be what the user wants. It doesn't handle parameters in text

It does, since that format was designed with that in mind :)

luanti/src/util/string.cpp

Lines 628 to 638 in 6949720

/* Translated strings have the following format:
* \x1bT marks the beginning of a translated string
* \x1bE marks its end
*
* \x1bF marks the beginning of an argument, and \x1bE its end.
*
* Arguments are *not* translated, as they may contain escape codes.
* Thus, if you want a translated argument, it should be inside \x1bT/\x1bE tags as well.
*
* This representation is chosen so that clients ignoring escape codes will
* see untranslated strings.

@appgurueu

Copy link
Copy Markdown
Contributor

Small inconvenience sure, but being guaranteed to return plain text is the entire point.

Related nit: I wonder whether there might be a more "intuitive" name than strip_escapes (what escapes?).

Escapes is a bit of a technical term, and one we're not even using that consistently (for example the translation-related methods do not use the term in their names).

What we want to express is that this turns our "rich" strings into "poor", "plain", "raw" (?) strings.

Something like core.to_plain_text maybe? Not sure.

@sfan5

sfan5 commented Sep 9, 2025

Copy link
Copy Markdown
Member Author

Something like core.to_plain_text maybe? Not sure.

someone could as well mistake that for dealing with hypertext...

@sfan5

sfan5 commented Sep 10, 2025

Copy link
Copy Markdown
Member Author

LuaJIT on CI:

(server) Time per loop: 2.0us
(server) [PASS] bench_strip_escapes - 2008ms
(server) Time per loop: 7.5us
(server) [PASS] bench_strip_escapes2 - 7561ms

Lua on CI:

(server) Time per loop: 20.0us
(server) [PASS] bench_strip_escapes - 20143ms
(server) Time per loop: 7.7us
(server) [PASS] bench_strip_escapes2 - 7712ms

LuaJIT on my machine:

(server) Time per loop: 1.6us
(server) [PASS] bench_strip_escapes - 1656ms
(server) Time per loop: 0.4us
(server) [PASS] bench_strip_escapes2 - 432ms

(strip_escapes2 is the C++ implementation)
Should I get rid of the Lua implementation then?

@appgurueu

Copy link
Copy Markdown
Contributor

Should I get rid of the Lua implementation then?

I'd say so, yes.

@appgurueu appgurueu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ok

@SmallJoker SmallJoker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sfan5
sfan5 merged commit d932f34 into luanti-org:master Sep 14, 2025
19 checks passed
@sfan5
sfan5 deleted the strip branch September 14, 2025 21:01
mjz19910 pushed a commit to mjz19910/luanti that referenced this pull request Mar 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants