-
Notifications
You must be signed in to change notification settings - Fork 8
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
Bugfix #69
Bugfix #69
Changes from 3 commits
3a24935
d42175f
2d3217b
b1d460b
7c8e881
e96c1a3
702cd2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,19 +5,29 @@ def map(csv_path:) | |
@headers = CSV.foreach(csv_path).first | ||
return unless valid?(csv_path: csv_path) | ||
translations = [] | ||
existing_key_for_label = {} | ||
|
||
CSV.foreach(csv_path, headers: true, skip_blanks: true) do |row| | ||
row_translations = map_row(row: row, locales: locales) | ||
next if row_translations.blank? | ||
|
||
existing_keys = translations.map(&:key) | ||
new_translations = row_translations.reject do |translation| | ||
existing_keys.any? do |key| | ||
existing_plural_key = key.label == translation.key.label && key.plural? && translation.key.singular? | ||
key.same_as?(key: translation.key) || existing_plural_key | ||
end | ||
current_key = row_translations.first.key | ||
|
||
current_label = current_key.label | ||
existing_key = existing_key_for_label[current_label] | ||
|
||
unless existing_key.nil? | ||
existing_plural_key = existing_key.label == current_key.label && existing_key.plural? && current_key.singular? | ||
existing_singular_key = existing_key.label == current_key.label && existing_key.singular? && current_key.plural? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tu peux ajouter 2 méthodes dans key ? Une same_singular_as? et l'autre same_plural_as? Ça va compléter l'interface de la classe, qui a déjà same_as? et ça simplifiera le code ici. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum tu vois cette implémentation pour tes deux méthodes :
Parce que vu le naming, j'aurais plutôt tendance à faire cette implémentation :
Or du coup ca ne correspond plus à la condition de mon code, qui vérifie que bien que les labels soient égaux, les singuliers / pluriels ne correspondent pas. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah bon point 🤔, je pensais à une implémentation équilivalente au code de existing_plural_key et existing_singular_key mais c'est vrai que same_**_as? évoque une égalité totale. Le naming ne convient pas. Tu en as un autre en tête ? J'ai pensé à same_label_as_singular?|same_label_as_plural? ou singular_with_same_label_as_plural?|plural_with_same_label_as_singular? ou singular_with_same_plural_as?| plural_with_same_singular_as? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum pas évident le naming ici. Je vais réfléchir. D'autant qu'au final ca me fait bizarre d'avoir cette méthode sur There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aussi, ça m'irait |
||
is_same_key = existing_key.same_as?(key: current_key) | ||
LOGGER.warn "A plural value already exist for key '#{current_label}'. Remove duplicates." if existing_plural_key | ||
LOGGER.warn "A singular value already exist for key '#{current_label}'. Remove duplicates." if existing_singular_key | ||
LOGGER.warn "Some values already exist for key '#{current_label}'. Remove duplicates." if is_same_key | ||
next if is_same_key || existing_plural_key || existing_singular_key | ||
end | ||
translations.concat(new_translations) | ||
|
||
existing_key_for_label[current_label] = current_key | ||
translations.concat(row_translations) | ||
end | ||
|
||
locale_wordings = translations.group_by(&:locale).map do |locale, group| | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,14 @@ def map(locale_wording:) | |
hash[translation.key.label] = {} unless hash.key? translation.key.label | ||
hash[translation.key.label][translation.key.plural_key] = translation.value | ||
else | ||
unless hash.is_a?(Hash) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment hash pourrait ne pas être un hash ? Ça voudrait dire que le code fait une mutation non voulue puisque hash est initialisé comme un hash avec le each_with_object({}) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ce ne serait pas hash[inner_key.to_s] que tu veux vérifier ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Une fois que tu as inséré la première clé et que tu passes dans la seconde, tu as
Donc quand tu arrives à There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah oui je vois, c'est la L25 qui fait la mutation de hash |
||
LOGGER.warn "Corrupted input. Trying to insert a value for key '#{translation.key.label}' but a value already exists for '#{inner_keys[0..index-1].join(".")}'. Skipping." | ||
break # skip this corrupted key | ||
end | ||
previous_value = hash[inner_key.to_s] | ||
if !previous_value.blank? && previous_value.is_a?(Hash) | ||
LOGGER.warn "Corrupted input. Trying to insert a value for key '#{translation.key.label}' but values already exist for keys '#{translation.key.label}.*'. Previous values will be lost." | ||
end | ||
hash[inner_key.to_s] = translation.value | ||
end | ||
else | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
"duplicate" = "Duplicate"; | ||
"singular_before_plural" = "Singular before Plural"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<plist> | ||
<dict> | ||
<key>plural_before_singular</key> | ||
<dict> | ||
<key>NSStringLocalizedFormatKey</key> | ||
<string>%#@key@</string> | ||
<key>key</key> | ||
<dict> | ||
<key>NSStringFormatSpecTypeKey</key> | ||
<string>NSStringPluralRuleType</string> | ||
<key>NSStringFormatValueTypeKey</key> | ||
<string>d</string> | ||
<key>one</key> | ||
<string>Plural before Singular</string> | ||
<key>other</key> | ||
<string>Plural before Singular</string> | ||
</dict> | ||
</dict> | ||
</dict> | ||
</plist> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
"duplicate" = "Duplicate"; | ||
"singular_before_plural" = "Singular before Plural"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<plist> | ||
<dict> | ||
<key>plural_before_singular</key> | ||
<dict> | ||
<key>NSStringLocalizedFormatKey</key> | ||
<string>%#@key@</string> | ||
<key>key</key> | ||
<dict> | ||
<key>NSStringFormatSpecTypeKey</key> | ||
<string>NSStringPluralRuleType</string> | ||
<key>NSStringFormatValueTypeKey</key> | ||
<string>d</string> | ||
<key>one</key> | ||
<string>Plural before Singular</string> | ||
<key>other</key> | ||
<string>Plural before Singular</string> | ||
</dict> | ||
</dict> | ||
</dict> | ||
</plist> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{"en":{"aaa":{"bbb":"first"},"bbb":{"ccc":"second"},"zzz":"first"}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
,key,fr,comment fr,en,comment en | ||
#Generic actions,,,,, | ||
,duplicate,Duplicate,,Duplicate, | ||
,duplicate,Erreur,,Error, | ||
#Plurals,,,,, | ||
,plural_before_singular##{one},Plural before Singular,,Plural before Singular, | ||
,plural_before_singular##{one},Plural before Singular,,Plural before Singular, | ||
,plural_before_singular##{other},Plural before Singular,,Plural before Singular, | ||
,plural_before_singular,Erreur,,Error,, | ||
,singular_before_plural,Singular before Plural,,Singular before Plural,, | ||
,singular_before_plural##{one},Erreur,,Error,, |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
,key,en, | ||
,aaa.bbb,first, | ||
,aaa.bbb.ccc,second, | ||
,bbb.ccc.ddd,first, | ||
,bbb.ccc,second, | ||
,zzz,first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tu peux utiliser un elsif ici, ça devrait donner le même résultat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bien vu, je corrige