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

Swiftgen and Clean up Localizable.strings #187

Merged
merged 8 commits into from
Nov 10, 2021
Merged

Conversation

PangMo5
Copy link
Member

@PangMo5 PangMo5 commented Oct 17, 2021

I apply Swiftgen for the new localization system.

The benefits written in Swiftgen repo.

  • Avoid any risk of typo when using a String
  • Free auto-completion
  • Avoid the risk of using a non-existing asset name
  • All this will be ensured by the compiler and thus avoid the risk of crashing at runtime.

Before

Text("Home")
Text("S\(String(item.parentIndexNumber ?? 0)):E\(String(item.indexNumber ?? 0))")

After

L10n.home.text
Text(L10n.seasonAndEpisode(item.parentIndexNumber ?? 0, item.indexNumber ?? 0))

For more information, see https://github.com/SwiftGen/SwiftGen.

  • Apply Swiftgen
  • Remove some unnecessary keys (ex: %@ • %@, Genres:)
  • Modify all localizable.string according to the new rule
  • Change all the string usage parts

If you have any additional opinions on localization, please give me.

I didn't touch the string, which was not translated before.

@PangMo5 PangMo5 self-assigned this Oct 17, 2021
@PangMo5 PangMo5 changed the title Apply R.swift Clean up Localizable.strings and R.swift Oct 17, 2021
@PangMo5 PangMo5 changed the title Clean up Localizable.strings and R.swift R.swift and Clean up Localizable.strings Oct 17, 2021
@PangMo5 PangMo5 added the enhancement New feature or request label Oct 17, 2021
# Conflicts:
#	JellyfinPlayer.xcodeproj/project.pbxproj
#	JellyfinPlayer.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	Translations/en.lproj/Localizable.strings
@PangMo5 PangMo5 added developer Developer facing issues and removed enhancement New feature or request developer Developer facing issues labels Nov 7, 2021
@PangMo5 PangMo5 mentioned this pull request Nov 7, 2021
@PangMo5 PangMo5 closed this Nov 7, 2021
@PangMo5 PangMo5 deleted the R.swift branch November 7, 2021 18:37
@PangMo5 PangMo5 restored the R.swift branch November 7, 2021 18:38
@PangMo5 PangMo5 reopened this Nov 7, 2021
@PangMo5 PangMo5 changed the title R.swift and Clean up Localizable.strings Swiftgen and Clean up Localizable.strings Nov 7, 2021
@PangMo5 PangMo5 marked this pull request as ready for review November 7, 2021 18:42
@PangMo5 PangMo5 requested a review from acvigue as a code owner November 7, 2021 18:42
@PangMo5 PangMo5 requested a review from LePips November 7, 2021 18:43
@PangMo5
Copy link
Member Author

PangMo5 commented Nov 7, 2021

I recommend merging this PR quickly before localizable.strings is updated due to the Weblate.

@jellyfin jellyfin deleted a comment from sonarcloud bot Nov 8, 2021
@jellyfin jellyfin deleted a comment from sonarcloud bot Nov 8, 2021
@LePips
Copy link
Member

LePips commented Nov 8, 2021

Just as dev note: I had trouble with CocoaPods since I'm now using an M1 machine and had to run some commands from this thread: CocoaPods/CocoaPods#10220

I will document this

@LePips
Copy link
Member

LePips commented Nov 8, 2021

Also, the generated file has swiftlint ignore rules. Because of this I'm almost convinced to just scrap swiftformat and keep to swiftlint

@LePips LePips mentioned this pull request Nov 8, 2021
@acvigue
Copy link
Member

acvigue commented Nov 8, 2021

Is this PR ready to merge?

@LePips
Copy link
Member

LePips commented Nov 9, 2021

It should be, yes

@PangMo5
Copy link
Member Author

PangMo5 commented Nov 9, 2021

Also, the generated file has swiftlint ignore rules. Because of this I'm almost convinced to just scrap swiftformat and keep to swiftlint

Oh, this is because I used the default template provided by Swiftgen.
I didn't have any other intentions.
We can also change it to SwiftFormat ignore rules if we want.
Is there any other reason to stop changing to SwiftFormat?

@LePips
Copy link
Member

LePips commented Nov 9, 2021

Is there any other reason to stop changing to SwiftFormat?

Oh no, I had just saw that I thought it would be a problem. Since it isn't, we're still good to go on SwiftFormat.

@PangMo5
Copy link
Member Author

PangMo5 commented Nov 9, 2021

Is this PR ready to merge?

Now it's ready 😄

@acvigue acvigue merged commit 0f78534 into jellyfin:main Nov 10, 2021
@sonarcloud
Copy link

sonarcloud bot commented Nov 10, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
12.3% 12.3% Duplication

@PangMo5 PangMo5 deleted the R.swift branch November 10, 2021 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer Developer facing issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Localize All User-Facing Strings Translation experience improvements
3 participants