Swiftfin icon indicating copy to clipboard operation
Swiftfin copied to clipboard

Clean up Localizations

Open LePips opened this issue 3 years ago • 6 comments
trafficstars

The base file Translations/en.lproj/Localizable.strings needs to be cleaned up as:

1 - there are many unused strings 2 - strings have just always been appended to the end of the file. This has lead to merge conflicts and has no organization.

So we need to find the unused strings, remove them, and then sort the remaining strings by some organization format. I was thinking sections by purpose and strings are alphabetized within each section:

/* General */
"close" = "Close";
...

/* QuickConnect */
"quickConnect" = "QuickConnect"
"quickConnectStep1" = "..."
...

As far as I know, re-organizing the strings does not cause Weblate to break.

LePips avatar Aug 08 '22 20:08 LePips

@LePips I can go through and do this. I was starting to work on this for the PR I was working on but it should be pretty straight forward for me to apply this to the other labels as well. I can make a PR for this, but does this layout make sense:

In-Progress Labels

Screenshot 2024-05-25 at 3 53 45 PM

At this time, my testing is that I do a search for the label before I organize it. If not found, I comment out labels and try a clean build for both tvOS and iOS to see if the label didn't show up in the original search on the project.

JPKribs avatar May 25 '24 21:05 JPKribs

As a note, the indentation does not interfere with the build. Let me know if this looks too busy with the indentation. This is what looked good to me but I'm game for whatever.

JPKribs avatar May 25 '24 21:05 JPKribs

Thank you for wanting to take a look at this. I like the comments for each string, but let's not indent anything.

The main issue that has prevented me from working on this over the years is determining how this interacts with Weblate/other languages and whether it will either move all of the strings or just nuke them. If it does the latter, then we cannot do this. If we have to manually perform the same movements in all files, then we'll probably need a script.

For determining which strings are unused, you can use one of the following which detects unused code and you can just see what it outputs for the Strings.swift file:

  • https://github.com/PaulTaykalo/swift-scripts
  • https://github.com/peripheryapp/periphery

LePips avatar May 25 '24 23:05 LePips

Thank you for wanting to take a look at this. I like the comments for each string, but let's not indent anything.

The main issue that has prevented me from working on this over the years is determining how this interacts with Weblate/other languages and whether it will either move all of the strings or just nuke them. If it does the latter, then we cannot do this. If we have to manually perform the same movements in all files, then we'll probably need a script.

For determining which strings are unused, you can use one of the following which detects unused code and you can just see what it outputs for the Strings.swift file:

* https://github.com/PaulTaykalo/swift-scripts

* https://github.com/peripheryapp/periphery

Sounds good. I can throw together a version of this and try testing for weblate. I'm assuming you mean this would break from the user side not from the build side? Otherwise, I can test to see if these localization are still active when I build and switch languages?

JPKribs avatar May 25 '24 23:05 JPKribs

Yes, the issue isn't building, but that we just don't lose translated strings.

LePips avatar May 26 '24 03:05 LePips

I'd be happy to do the re-organization and then, if the translations wouldn't map, we can toss the PR. I'm just looking for things I can help with while I'm getting better at SwiftUI on the side. So no hard feelings from me if this doesn't work out

JPKribs avatar May 26 '24 05:05 JPKribs

I was going to make another issue for looking at this then I remembered this one! I am the biggest offender for this but I wanted to discuss before I started making changes. Primarily, I think fixing up some of the casing is a decent place to start:

Image

I can go in and start getting some of our English localizations worked out to be more consistent but I wanted to make sure I was doing this right.

Option 1

Handle all localizations in lowercase, or titlecase or uppercase, whichever is the best for storage. Then, I know SwiftUI has some functions for Strings like:

  • localizedCapitalized
  • localizedUppercase
  • localizedLowercase
  • uppercased
  • lowercased

That we could use based on usage?

Option 2

Move all localizations to Sentence case or whatever our most common usage will be. Then, as needed, we can create Title localizations for instances where we need Title Case Strings vs Sentence case strings .


My thought it, when we feel ready to proceed, I can knock out localization organizing, deleting unused ones, and get casing knocked out all at once.

Let me know what you think is best (and when would be a good time to look at this again). No rush at all!

JPKribs avatar Dec 12 '24 21:12 JPKribs

I've scripted this out in Python. Well, ChatGPT and I scripted this out in Python... But, I am doing:

  1. Find all usages of a localization
  2. Find all strings that have existing localizations
  3. Find all strings that do not have localizations
  4. Create a new Localizable.strings that is alphabetized and looks like this:
/// Who's watching?
/// Total Usages: 0
"WhosWatching" = "Who's watching?";

/// About
/// Total Usages: 4
"about" = "About";

/// Absolute
/// Total Usages: 1
"absolute" = "Absolute";
...
  1. Create a log file that contains all strings that have localizations and where they should go:
Video Player Type (use L10n.videoPlayerType instead):
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Shared/Strings/Strings.swift (Line 1418)

The video profile is not supported (use L10n.videoProfileNotSupported instead):
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Shared/Strings/Strings.swift (Line 1420)

The video range type is not supported (use L10n.videoRangeTypeNotSupported instead):
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Shared/Strings/Strings.swift (Line 1422)

Video remuxing (use L10n.videoRemuxing instead):
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Shared/Strings/Strings.swift (Line 1424)

The video resolution is not supported (use L10n.videoResolutionNotSupported instead):
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Shared/Strings/Strings.swift (Line 1426)
...
  1. Create a log of ALL strings in Swiftfin and where they are located:
"Double Touch" found at:
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Swiftfin/Views/SettingsView/GestureSettingsView.swift (Line 51)

"Download" found at:
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Swiftfin/Views/DownloadTaskView/DownloadTaskContentView.swift (Line 51)

"Download task does not have media url for item: \(downloadTask.item.displayTitle)" found at:
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Shared/ViewModels/VideoPlayerManager/DownloadVideoPlayerManager.swift (Line 18)

"Downloaded items are only playable through the Swiftfin video player." found at:
  - /Users/jpkribs/Projects/StringTest/Swiftfin/Swiftfin/Views/DownloadTaskView/DownloadTaskContentView.swift (Line 119)
...

This makes it very easy whenever we're ready to look at localizations. My only question is do we like that Localizable.strings format where it's just the String and a count of usages?

~Script attached.~ Nvm it's probably smart that you can't attach .pys like that. Script available on request!

JPKribs avatar Dec 13 '24 16:12 JPKribs

Option 1 is best since not everything should necessarily be lowercased since proper nouns, like Jellyfin, should be capitalized. Like you've noted, for places where we would still need title case, like section or navigation headers, we would use the localizedCapitalized helpers.

However we still have discretion over what counts as a proper noun (thing). For example, in that screenshot:

  • Random Image -> Random image, since image is not a proper noun
  • Favorites, Next Up and Recently Added are names for sections of media, and should stay capitalized. Depending on where these names may be in the strings we wouldn't be able to use localizedCapitalized or other modifiers if these are all sentence case.

I do like the organization provided by that script but would not include the count of usages. My only consideration for usages would be whether to remove unused strings.

LePips avatar Dec 13 '24 19:12 LePips

Like you've noted, for places where we would still need title case, like section or navigation headers, we would use the localizedCapitalized helpers.

Literally my only issue localizedCapitalized is TV. Since it turns TV Shows into Tv Shows. That is a very very very minor reason to not like this haha.

I just committed https://github.com/jellyfin/Swiftfin/pull/1362 which removed unused localizations and alphabetizes all used localizations. I think that's a decent starting point where I can manually go in and start putting everything in sentence case with proper nouns.

Either that, or I have a list of "Strings" that I can start localizing all the hardcoded values?

Also, do we want to store the script anywhere if we need it later? I was not super hard to do so I don't feel supper attached to it and I don't know if we'll need to rerun this in the future.

Lastly, can we just do a viewModifer that uses localizedCapitalized for .navigationTitle or will that need to be updated Since that might screw up movies/shows ItemView

JPKribs avatar Dec 13 '24 20:12 JPKribs