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

Partial support for Translations #207

Merged
merged 7 commits into from
May 9, 2020
Merged

Partial support for Translations #207

merged 7 commits into from
May 9, 2020

Conversation

n76
Copy link
Contributor

@n76 n76 commented May 5, 2020

Partial implemented support for the languages and dialets that Roku supports.

A number of structural issues encountered the biggest are:

  1. Programatically catenating strings which assumes a particular language syntax.
  2. Using display strings in buttons, etc. to determine action.
  3. Building display objects in BrightScript rather than in XML. This results in many cases where you have to translate the same string many times (e.g. "Cancel")

Side note: My editor is set to remove trailing spaces and to assure a new line at the end of the file. Existing source code seems to vary on this. I can revert those few changes if that is a problem.

Finally, added https://reuse.software/spec/ compliant license information to the new files and the files I touched.

Signed-off-by: Tod Fitch tod@fitchfamily.org

Changes

Issues

Partial implemented support for the languages and dialets that Roku supports.

A number of structural issues encountered the biggest are:
1. Programatically catenating strings which assumes a particular language syntax.
2. Using display strings in buttons, etc. to determine action.
3. Building display objects in BrightScript rather than in XML. This results in many cases where you have to translate the same string many times (e.g. "Cancel")

Side note: My editor is set to remove trailing spaces and to assure a new line at the end of the file. Existing source code seems to vary on this. I can revert those few changes if that is a problem.

Finally, added https://reuse.software/spec/ compliant license information to the new files and the files I touched.

Signed-off-by: Tod Fitch <tod@fitchfamily.org>
@neilsb
Copy link
Member

neilsb commented May 6, 2020

That's a great start, and the structural issue you highlighted are things we should look at addressing before the client grows, which will just compound the problem.

The REUSE Specification is not something I've come across before. I'm not sure if we want that at the top of every file, but certainly happy to have a discussion on it. Might I suggest that we move that into it's own PR or issue? And if we're going to do it we should probably hit all the files at the one time and then it becomes the standard. On thing, though, is that I believe the Jellyfin project, and the roku client both use GPL v2, as opposed to GPL v2 or later, just looking at the current license file.

@anthonylavado anthonylavado changed the title Partial support for https://github.com/jellyfin/jellyfin-roku/issues/36 Partial support for Translations May 6, 2020
@anthonylavado
Copy link
Member

For licensing, yes, let's please move that in to a separate PR if we can?

I've edited the title to reflect what I believe was the intention of this PR.

It's 5 am here, so when I'm properly awake I'll look over the changes for translation. We use Weblate for the rest of the project, so if we can set up string files in a compatible/supported format, I can add this repository to the software, enabling better translation support.

@neilsb
Copy link
Member

neilsb commented May 6, 2020

One for the formats Roku supports is XLIFF, which is what @n76 has used. XLIFF is also conveniently supported by Weblate according to their docs. So that's positive.

Signed-off-by: Tod Fitch <tod@fitchfamily.org>
@n76
Copy link
Contributor Author

n76 commented May 6, 2020

I've removed the license info headers and fixed the 'Ends at ' issue. At least for movies. I am having issues that I believe are unrelated to my changes in viewing detailed data for TV shows and other collections.

There is another case similar to the 'Ends at ' for entering settings in components/config/ConfigList.brs at line 51: dialog.title = "Enter the " + configField.label. The problem being that the configField.label has already been translated. But simply fixing this the same as 'Ends at ' assumes the same word order, declensions, etc. as English. I think this will have to be refactored to be pass the full string in the object.

Signed-off-by: Tod Fitch <tod@fitchfamily.org>
@cewert
Copy link
Member

cewert commented May 6, 2020

@n76 curious what made you go with the XLIFF format?

It looks like we can choose either TS or XLIFF. I don't know all the differences between the two but I noticed XLIFF uses ID's and TS doesn't. So to me TS seems easier to maintain since we wouldn't have to worry about the ID but I assume there's other factors to consider as well.

@n76
Copy link
Contributor Author

n76 commented May 6, 2020

@n76 curious what made you go with the XLIFF format?

It looks like we can choose either TS or XLIFF. I don't know all the differences between the two but I noticed XLIFF uses ID's and TS doesn't. So to me TS seems easier to maintain since we wouldn't have to worry about the ID but I assume there's other factors to consider as well.

I used XLIFF on this because I'd used it on my PeerTube Roku channel. Now I have to remember why I used it there instead of TS. And I think it was just a toss of the coin. I don't know how critical the ID numbers are in XLIFF as far as Roku is concerned and admit they are a bit of a hassle to keep up. If this is an issue, I'll look into converting the XLIFF to TS.

By the way, writing that PeerTube channel was inspired by seeing this project. And also what got me interested in the licensing issue as I borrowed the make files and a utility file from here but the licensing I used was different from GPL2 only so I need a way to keep the GPL2 only licensing on the borrowed files. That turned out useful when I needed a custom font (GPL3 or later) and some icons (Apache 2.0).

@n76
Copy link
Contributor Author

n76 commented May 6, 2020

Conversion from XLIFF to TS was pretty simple and seems to work the same. I know almost nothing of Weblate but it looks like it can deal with TS files as well as XLIFF files.

Should I update the pull request to use TS?

@neilsb
Copy link
Member

neilsb commented May 7, 2020

Seems that either should be fine with Webalate, but if using TS means we don't need to manually manage an id, then I'd say that would be preferable.

Signed-off-by: Tod Fitch <tod@fitchfamily.org>
@neilsb
Copy link
Member

neilsb commented May 8, 2020

That's great. One thing we'll need to be aware of now is when laying things out a label, title, button, etc may be a different size depending on the language.

There is only one example of this I found currently, and that's when you're using German. The "* Options" on the JFOverhang is now right up against the right hand edge of the screen. I suspect this could be solved using a right aligned layout group or something. Might be worth sorting this out before merging.

@n76
Copy link
Contributor Author

n76 commented May 8, 2020

That's great. One thing we'll need to be aware of now is when laying things out a label, title, button, etc may be a different size depending on the language.

There is only one example of this I found currently, and that's when you're using German. The "* Options" on the JFOverhang is now right up against the right hand edge of the screen. I suspect this could be solved using a right aligned layout group or something. Might be worth sorting this out before merging.

The "* Options" positioning is complicated by the fact that it is two different text strings and the sizes are different (set different in the XML and then changed in the BRS). My initial searches to see if you can vary font size, etc. within a string have come up blank. Next search was to see if one element position can be made relative to another. First search on that has come up blank too. My next search was to see if there was a call to determine the actual display width of text (then the "*" could be repositioned to be just to the left of the "Options" text). No luck there either.

Not to say that any of those things can't be done as I've had trouble finding things in the Roku documents before. Just that the searches I made for the first few ways to address this that I thought of came up empty. It looks like Roku themselves know how to do it if you look at the time display, I just haven't found the way yet.

In the meantime, I'll push a change where I manually moved the "* Options" to the left by a few pixels. Making it look good in one language does upset the look a little for other languages.

Move display a bit to left to account for longer string
when language set to German.

Signed-off-by: Tod Fitch <tod@fitchfamily.org>
components/JFOverhang.xml Outdated Show resolved Hide resolved
Signed-off-by: Tod Fitch <tod@fitchfamily.org>
Copy link
Member

@neilsb neilsb left a comment

Choose a reason for hiding this comment

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

That's all looking great now. A significant feature knocked off the list. Thankyou. Great work.

I'll let @cewert give it a final once over before merging.

Copy link
Member

@cewert cewert left a comment

Choose a reason for hiding this comment

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

Great work on this!

@cewert cewert merged commit 86d4b2d into jellyfin:master May 9, 2020
@n76 n76 deleted the locale branch May 9, 2020 04:14
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.

None yet

4 participants