Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: translate fallback chapter title #540

Merged
merged 2 commits into from
Apr 28, 2023

Conversation

po5
Copy link
Contributor

@po5 po5 commented Apr 27, 2023

As well as handling of (unnamed) for mkv demuxer, empty string for disc demuxer.
https://github.com/mpv-player/mpv/blob/c5211dbf4ae38583b4a55ab63c5c07f8547f73b8/demux/demux_mkv.c#L964
https://github.com/mpv-player/mpv/blob/c5211dbf4ae38583b4a55ab63c5c07f8547f73b8/demux/demux_disc.c#L285
Other demuxers either don't supply a title, or use NULL as a fallback value.

Took care of German and Simplified Chinese too, since those have verified translations on Google.

@po5
Copy link
Contributor Author

po5 commented Apr 27, 2023

Should we mangle existing chapter titles if they follow the ^Chapter %d+$ format?
It's common for files to have these chapters with useless titles, and it's weird to keep them untranslated.
We wouldn't risk anything bad like accidental renumbering, just match ^Chapter %d+$, override with Chapter %s, and provide string.match(chapter.title, '%d+') as t() param.

Done, cherrypick commits if you disagree.

@christoph-heinrich
Copy link
Contributor

Can confirm that the German translation is correct.

Chapter translations can even be taken a step further by also translating common chapter names like "Intro".
I'm not sure if that's desirable tho.

@po5 po5 force-pushed the intl-chapter-title branch 2 times, most recently from 0b2ecf8 to 3200635 Compare April 27, 2023 14:24
@tomasklaen tomasklaen merged commit 8561861 into tomasklaen:main Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants