Reverted to accept string value in strings file of all languages #3246
Conversation
📝 WalkthroughWalkthroughThe changes update multiple localization files by replacing integer ("%1$d") and floating-point ("%1$.2f") format specifiers with generic string ("%s" or "%1s") specifiers in several string resources. This affects messages related to application limits, recovery form submission counts, and area formatting across Spanish, French, Portuguese, Swahili, Tigrinya, and default English resources. In the Spanish strings, the area unit is also changed from "m²" to "sq m". No changes to the actual textual content or application control flow are present; only the format specifiers for value substitution are modified. Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (13)
app/res/values-ti/strings.xml (2)
28-28: Prefer indexed string placeholder (%1$s) for translator-safe orderingUsing a plain
%sremoves the positional index that Android translators rely on when sentence structure differs per-locale.
Replace%swith%1$sto keep the fix while preserving ordering flexibility.- <string name="multiple_apps_limit_message">… %s …</string> + <string name="multiple_apps_limit_message">… %1$s …</string>
165-167: Adopt positional specifier in recovery strings as wellSame rationale as above – switch to
%1$sso that future translations can reorder arguments without code changes.- <string name="recovery_forms_send_successful">… %s …</string> - … - <string name="recovery_forms_send_parital_success">… %s …</string> + <string name="recovery_forms_send_successful">… %1$s …</string> + … + <string name="recovery_forms_send_parital_success">… %1$s …</string>app/res/values-sw/strings.xml (2)
40-40: Use%1$sinstead of bare%sfor placeholder consistencySee Tigrinya comment – keep positional index to help translators.
- <string name="multiple_apps_limit_message">… %s …</string> + <string name="multiple_apps_limit_message">… %1$s …</string>
178-180: Align recovery placeholders with Android formatting guidelinesReplace both
%soccurrences with%1$s.- <string name="recovery_forms_send_successful">… %s …</string> - … - <string name="recovery_forms_send_parital_success">… %s …</string> + <string name="recovery_forms_send_successful">… %1$s …</string> + … + <string name="recovery_forms_send_parital_success">… %1$s …</string>app/res/values-pt/strings.xml (2)
40-40: Retain argument index in limit message- <string name="multiple_apps_limit_message">… %s …</string> + <string name="multiple_apps_limit_message">… %1$s …</string>
177-179: Positional placeholder missingUpdate both recovery strings:
- <string name="recovery_forms_send_successful">… %s …</string> - … - <string name="recovery_forms_send_parital_success">… %s …</string> + <string name="recovery_forms_send_successful">… %1$s …</string> + … + <string name="recovery_forms_send_parital_success">… %1$s …</string>app/res/values/strings.xml (2)
49-49: Consider using apluralsresource for app-count messages
multiple_apps_limit_messagehard-codes a single placeholder (%s apps) inside a plain<string>.
Because the value represents a quantity, Android’spluralsresource should be used so the wording and grammar can change automatically for “1 app” vs “2 apps”, and translators get the correct context.No functional bug today, but this will surface in some languages (e.g. Russian) where singular/dual/plural forms differ.
217-220: Same quantity → same concern aboutpluralsBoth
recovery_forms_send_successfulandrecovery_forms_send_parital_successembed a numeric count inside a simple<string>. Use<plurals>to avoid awkward “1 forms were” constructions and give translators correct plural rules.app/res/values-fr/strings.xml (2)
40-40: Plural handling – same recommendation as base language
multiple_apps_limit_messagenow uses%sfor the count. Prefer a<plurals>entry so French translators can distinguish “1 application” vs “2 applications”.
177-179: Plural handling – see above
recovery_forms_send_successful/recovery_forms_send_parital_successshould move to apluralsresource to avoid incorrect agreement (“Tous les formulaires 1 ont été soumis”).app/res/values-es/strings.xml (3)
40-40: Use%1$sto keep placeholder ordering locale-safeRelying on plain
%sremoves the positional index that Android’s formatter uses to reorder arguments for other locales. Safer to keep%1$s(or%1$dif the value is always numeric) so translators can reshuffle placeholders without breaking the code.- <string name="multiple_apps_limit_message">Has alcanzado tu límite de aplicaciones instaladas; solo puedes tener %s aplicaciones instaladas a la vez... + <string name="multiple_apps_limit_message">Has alcanzado tu límite de aplicaciones instaladas; solo puedes tener %1$s aplicaciones instaladas a la vez...
115-115: Missing positional index & awkward wordingSame indexing issue as above (
%1$s).
Additionally, “los formularios %s” reads oddly in Spanish—consider “los %1$s formularios”.- <string name="recovery_forms_send_successful">Envío exitoso. Se enviaron todos los formularios %s.</string> + <string name="recovery_forms_send_successful">Envío exitoso. Se enviaron los %1$s formularios.</string>
117-117: Indexing & typo
- Placeholder index again (
%1$s).- Key already contains a typo:
parital. While outside the diff scope, correcting it now prevents future confusion.- <string name="recovery_forms_send_parital_success">Ocurrieron errores ... Solo se enviaron %s... + <string name="recovery_forms_send_partial_success">Ocurrieron errores ... Solo se enviaron %1$s...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/res/values-es/strings.xml(3 hunks)app/res/values-fr/strings.xml(3 hunks)app/res/values-pt/strings.xml(3 hunks)app/res/values-sw/strings.xml(3 hunks)app/res/values-ti/strings.xml(3 hunks)app/res/values/strings.xml(3 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
Learnt from: shubham1g5
PR: dimagi/commcare-android#2955
File: app/src/org/commcare/models/database/user/models/CommCareEntityStorageCache.java:135-143
Timestamp: 2025-02-18T17:42:31.516Z
Learning: In CommCare Android, record IDs are expected to be numeric and should fail fast (crash) if a non-numeric ID is encountered, rather than handling the NumberFormatException.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2912
File: app/src/org/commcare/fragments/connect/ConnectPaymentSetupFragment.java:61-66
Timestamp: 2025-01-21T17:29:58.014Z
Learning: In the CommCare Android app, for non-critical convenience features like phone number auto-population, exceptions should be logged but fail silently when there's a manual fallback available. This approach prevents app crashes while maintaining the ability to debug issues through logs.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/res/values-ti/strings.xml:350-350
Timestamp: 2025-05-22T14:32:53.133Z
Learning: PersonalID and Connect features haven't been translated to Spanish, Lithuanian, or Norwegian yet, so users with those language settings see the English strings by default.
app/res/values-sw/strings.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
app/res/values/strings.xml (3)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
app/res/values-fr/strings.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
app/res/values-ti/strings.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/values-es/strings.xml (2)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/values-pt/strings.xml (1)
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
| <string name="audio_download_prompt">ንዚ ሕቶ ፋይል ድምፂ ዝጎደሎ እዩ፤ ዳታ ንምውራድ ነዚ ልዕል ኢሉ ዝርከብ ምልክት ዳታ መውርዲ ጠውቅ/ፅቀጥ</string> | ||
| <string name="media_not_found_error">ኣብዚ መተግበሪ ሚድያ (መራክቢ) ኣይተረክበን</string> | ||
| <string name="area_format">%1$.2f sq m</string> | ||
| <string name="area_format">%1s sq m</string> |
There was a problem hiding this comment.
Missing ‘$’ after argument index – current spec is parsed as width, not index
%1s sets a minimum width of 1 and does not reference “argument 1”.
Use %1$s to correctly reference the first argument and avoid accidental padding / future crashes.
- <string name="area_format">%1s sq m</string>
+ <string name="area_format">%1$s sq m</string>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <string name="area_format">%1s sq m</string> | |
| <string name="area_format">%1$s sq m</string> |
🤖 Prompt for AI Agents
In app/res/values-ti/strings.xml at line 214, the format specifier %1s is
incorrectly used as it sets a minimum width instead of referencing the first
argument. Change %1s to %1$s to correctly reference the first argument in the
string format and prevent unintended padding or runtime errors.
| <string name="audio_download_prompt">Faili ya sauti haipo kwa swali hili, bofya kitufe cha kupakua hapo juu ili kupakua</string> | ||
| <string name="media_not_found_error">Midia haipatikani kwenye programu</string> | ||
| <string name="area_format">%1$.2f sq m</string> | ||
| <string name="area_format">%1s sq m</string> |
There was a problem hiding this comment.
Incorrect format specifier – add ‘$’ after index
Same width-vs-index issue as in TI file.
- <string name="area_format">%1s sq m</string>
+ <string name="area_format">%1$s sq m</string>🤖 Prompt for AI Agents
In app/res/values-sw/strings.xml at line 225, the format specifier %1s should
include a '$' after the index to correctly reference the argument position.
Change %1s to %1$s to fix the width-vs-index formatting issue.
| <string name="audio_download_prompt">O arquivo de áudio está faltando para esta pergunta, clique no botão de download acima para fazer o download</string> | ||
| <string name="media_not_found_error">Mídia não encontrada no aplicativo</string> | ||
| <string name="area_format">%1$.2f m²</string> | ||
| <string name="area_format">%1s m²</string> |
There was a problem hiding this comment.
Fix specifier: %1s ⇒ %1$s
Without the ‘$’, %1s is treated as width = 1, which happens to print correctly today but is error-prone.
- <string name="area_format">%1s m²</string>
+ <string name="area_format">%1$s m²</string>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <string name="area_format">%1s m²</string> | |
| <string name="area_format">%1$s m²</string> |
🤖 Prompt for AI Agents
In app/res/values-pt/strings.xml at line 226, the format specifier %1s should be
corrected to %1$s to explicitly indicate the argument index. Replace %1s with
%1$s in the string resource to ensure proper and safe string formatting.
| <string name="audio_download_prompt">Audio file is missing for this question, click the download button above to download</string> | ||
| <string name="media_not_found_error">Media not found in the application</string> | ||
| <string name="area_format">%1$.2f sq m</string> | ||
| <string name="area_format">%1s sq m</string> |
There was a problem hiding this comment.
Formatting bug – missing $ in positional specifier
%1s is parsed as width 1, not as “first argument” – so the argument index is lost.
This compiles, but Android will treat the 1 as a width flag and may truncate values unexpectedly.
- <string name="area_format">%1s sq m</string>
+ <string name="area_format">%1$s sq m</string>If only one argument is ever supplied you can simplify to %s sq m.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <string name="area_format">%1s sq m</string> | |
| <string name="area_format">%1$s sq m</string> |
🤖 Prompt for AI Agents
In app/res/values/strings.xml at line 277, the format specifier %1s is incorrect
because it is interpreted as a width specifier rather than a positional
argument. To fix this, either change the format specifier to %1$s to correctly
indicate the first argument or simplify it to %s if only one argument is used.
This will ensure the string formatting works as intended without truncation.
| <string name="audio_download_prompt">Le fichier audio est manquant pour cette question, cliquez sur le bouton de téléchargement ci-dessus pour télécharger</string> | ||
| <string name="media_not_found_error">Média non trouvé dans l\'application</string> | ||
| <string name="area_format">%1$.2f m²</string> | ||
| <string name="area_format">%1s m²</string> |
There was a problem hiding this comment.
Formatting bug – missing $ in positional specifier
%1s m² should be %1$s m² (or just %s m²).
Without the $, the 1 is interpreted as width, leading to unexpected truncation.
- <string name="area_format">%1s m²</string>
+ <string name="area_format">%1$s m²</string>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <string name="area_format">%1s m²</string> | |
| <string name="area_format">%1$s m²</string> |
🤖 Prompt for AI Agents
In app/res/values-fr/strings.xml at line 224, the format specifier `%1s` is
incorrect because it lacks the `$` sign, causing the `1` to be treated as a
width specifier rather than a positional argument. Fix this by changing `%1s` to
`%1$s` to correctly indicate the first argument as a string, or simply use `%s`
if positional specifiers are not needed.
| <string name="audio_download_prompt">Falta el archivo de audio para esta pregunta, haz clic en el botón de descarga de arriba para descargar</string> | ||
| <string name="media_not_found_error">Medio no encontrado en la aplicación</string> | ||
| <string name="area_format">%1$.2f m²</string> | ||
| <string name="area_format">%1s sq m</string> |
There was a problem hiding this comment.
Accidental English unit & incorrect format spec
The Spanish file now shows “sq m” (English) and %1s (width = 1) instead of %1$s.
This regresses localisation and could print an unexpected unit.
- <string name="area_format">%1s sq m</string>
+ <string name="area_format">%1$s m²</string>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <string name="area_format">%1s sq m</string> | |
| <string name="area_format">%1$s m²</string> |
🤖 Prompt for AI Agents
In app/res/values-es/strings.xml at line 162, the string resource uses the
English unit "sq m" and an incorrect format specifier "%1s". Replace "sq m" with
the correct Spanish unit equivalent and change the format specifier to "%1$s" to
properly format the string argument for localization.
|
@damagatchi retest this please |
Product Description
Facing bug with bellow stack trace
Technical Summary
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review