Conversation
|
@Kharchevskyi many ui tests are failing after update - https://flowcrypt.semaphoreci.com/jobs/3d7e754c-290c-46fd-a54f-1c16eb60eb66/summary?test-result-id=4ae71336-e44b-39bf-b9d2-752e234818a5 |
|
@sosnovsky thanks, already on it. Checking |
sosnovsky
left a comment
sosnovsky
left a comment
There was a problem hiding this comment.
Looks good, just need to fix ui tests and use stringsdict for pluralisation support
| userInfoMessage = "import_no_backups_clipboard".localized + user | ||
| } else { | ||
| userInfoMessage = "Found \(privateKey.count) key\(privateKey.count > 1 ? "s" : "")" | ||
| let key = privateKey.count > 1 |
There was a problem hiding this comment.
For such cases it's better to use stringsdict with pluralisation support - it'll automatically use appropriate localization for provided parameter (privateKey.count) - https://github.com/FlowCrypt/flowcrypt-ios/blob/master/FlowCrypt/Resources/Localizable.stringsdict
There was a problem hiding this comment.
Why I do not prefer this solution because NSLocalizedString accepts array of CVarArg as arguments and in case we pass string instead of int we will get a crash in runtime
As you see to prevent that I have changed implementation for localizeWithArguments to accept only stings in code and all parameters in localization.strings accepts only %@ as String parameter
@inline(__always) func localizeWithArguments(_ arguments: String...)
With stringsdict solution we need to explicitly pass Int as a variable to be able to distinguish is it variable plural.
Do we really want to have this possibility?
There was a problem hiding this comment.
@sosnovsky
What do you think if I leave only Strings as parameters here in localizeWithArguments implementation? And we will use only string as a parameter in localizable files
And will add a separate func localizePluralWithArguments to accept Int and use stringsdict?
There was a problem hiding this comment.
@sosnovsky What do you think if I leave only Strings as parameters here in
localizeWithArgumentsimplementation? And we will use only string as a parameter in localizable filesAnd will add a separate
func localizePluralWithArgumentsto acceptIntand use stringsdict?
Sounds good for me, let's do it this way 👍
| switch subtitleType { | ||
| case let .fetchedKeys(count): | ||
| subtitle = "Found \(count) key backup\(count > 1 ? "s" : "")" | ||
| let key = count > 1 ? "setup_fetched_keys" : "setup_fetched_key" |
There was a problem hiding this comment.
stringsdict should be used here too
2 participants
This PR moves texts to separate strings files.
@inline(__always) func localizeWithArguments(_ arguments: String...) -> Stringchanged to accept only strings arguments to prevent crashes in runtimeclose #956
Tests (delete all except exactly one):
To be filled by reviewers
I have reviewed that this PR... (tick whichever items you personally focused on during this review):