Prevously, the translation update script would match if not input:find("%S") then because S" is a valid function call expression.

I am confused: S" is not a valid function call expression? Do you mean: If there is another string later, S")..." could be a valid function call expression?

Either way the problem is that we start matching inside a string, no? The only real way to combat that is to keep track of tokens (at least at a string / non-string granularity), then have a regex of (token)*pattern.

Is there ever a situation where we want to allow empty translation strings?

(Ultimately this is another problem caused by just throwing regex at this rather than a clean two step approach starting with tokenization... but probably not worth fixing?)

Do you mean: If there is another string later, S")..." could be a valid function call expression?

Yes, that. The code I wrote to fix that is not perfect, but at least gets rid of all false-positives within luanti/builtin.

Either way the problem is that we start matching inside a string, no? The only real way to combat that is to keep track of tokens (at least at a string / non-string granularity), then have a regex of (token)*pattern.

Yes, but I'm not going to implement that. Regex isn't the best choice here, but it's what we've got, and what works OK in most cases. Feel free to propose code changes.

Is there ever a situation where we want to allow empty translation strings?

Modders's choice to provide convenience for the translators.

This furthermore adds the option '--discard-empty' which omits empty translation strings from all files but template.txt.

Usecase? Empty strings in translation files allow looking for strings that are not yet translated and translating those directly (without cross-referencing the template or adding the missing entries manually).

Discarding empty strings brings hardly any gain to speak of in terms of disk space unless many strings are not translated. The argument on disk space/network transfer times is also somewhat paradoxical in itself: the "wasted" space is a result of the lack of translations; this should arguably be improved by actually providing translations for such strings, which has the opposite effect of increasing the size of translations files.

(Edit: Clarification)

Oh! I had misunderstood "empty translation strings" to mean S(""). Should have taken a closer look at the code. Disregard my above comment regarding that then, it was related to my question as to which function call expression was being referred to...

Removed the --discard-empty option.

Oh! I had misunderstood "empty translation strings" to mean S("").

Actually there should also be a warning against S("") (perhaps in a separate PR or a warning in the engine¹?). PO files use the empty source string for metadata, so people who use the empty source string in tr files and (later) convert them to PO will have quite a surprise at some point.