TextFileReader: strip leading invisible Unicode characters on first line only

[niaBaldoni]

Fixes #614

Problem

Subtitle files generated by tools like faster-whisper might sometimes contain unexpected invisible Unicode characters (e.g. U+200E LEFT-TO-RIGHT MARK, U+202A LEFT-TO-RIGHT EMBEDDING) at the very start of the file. These cause SRT parsing to fail with an error:

Parsing SRT: Expected subtitle index at line 1

The SRT parser's digit check fails because the invisible character precedes the subtitle index on the first line. Since the characters are invisible in common text editors, the user has no way of knowing what went wrong.

Changes

The existing U+FEFF (BOM) check ran on every line of every file. This has been moved into a first_line guard so it only runs once. The guard now also strips a broader set of invisible Unicode characters before any format parser sees the first line. Characters within subtitle content (such as RTL marks, possibly present in Arabic or Persian subtitles) are intentionally preserved, since the stripping is scoped to the first line only.

Tests

Added tests/tests/text_file_reader.cpp:

• strips_bom_on_first_line: existing BOM behaviour is preserved
• strips_leading_invisible_char: single invisible character at file start is stripped
• strips_stacked_leading_invisible_chars: multiple stacked invisible characters are all stripped
• preserves_invisible_chars_in_content: U+200F inside subtitle content is not stripped

Test files in tests/text_file_reader/.

[arch1t3cht]

This issue involves some tricky philosophical decisions, so I will write out my thoughts here as best as I can:

First of all:

Subtitle files generated by tools like faster-whisper might sometimes contain unexpected invisible Unicode characters (e.g. U+200E LEFT-TO-RIGHT MARK, U+202A LEFT-TO-RIGHT EMBEDDING) at the very start of the file.

Independently from whatever Aegisub does, this feels like it warrants further investigation. Why exactly does faster-whisper generate these characters? Is this a bug in faster-whisper or is it somehow intended? If it's the former, this should be reported and fixed there. If it's the latter, other tooling like Aegisub might need to handle this in some other way than just stripping these characters.

---

Next, assuming that such files are indeed "invalid", there is still the debate of how Aegisub should handle such files.

In general, my belief is that tooling should strictly enforce specification-compliant input rather than silently apply implicit fixes and guessing. (See also: The Harmful Consequences of the Robustness Principle.) As such, I disagree with silently implicitly stripping any leading white-space (unless you can point to a specification that deems this valid).

However, as you also point out on Discord, Aegisub is an editor, and hence its job is to allow users to edit subtitle files. In particular, it should allow users to recover and fix and invalid subtitle files. So I definitely agree that the current situation where Aegisub plainly refuses to open such a file with a fairly crytic error message is not good.

Hence, I think that the "ideal" policy would be one where

• Aegisub ensures that all the files it loads are fully specification-compliant
• However, when encountering noncompliant files it will not reject them outright. Instead, it will
  • Alert the user with detailed error messages indicating the offending parts
  • Offer to recover the file (or do so automatically, but only after informing the user) by e.g. automatically stripping any incorrect sections.

(Of course, an additional problem here is that many specifications for subtitle formats are murky at best and nonexistent at worst, but this doesn't change the underlying philosophy too much.)

Following this policy fully will probably need big refactors to all of Aegisub's parsing code, but until that happens any smaller changes to Aegisub's parsing should at least move towards this direction where possible.

---

Finally, I believe that doing this stripping of leading whitespace in TextFileReader is incorrect either way: TextFileReader is supposed to be a format-agnostic reader for text files, and unconditionally stripping leading white space in that is bound to cause bad surprises at some point. For example, when loading a raw txt file, it'd certainly possible for some lines to start with an RTL override, and stripping those automatically would be incorrect.

So, either way, this detection and/or stripping should be done at the level of the individual subtitle formats instead (possibly using a centralized utility function if necessary).