Skip to content

[FEATURE REQUEST] Add action to check source strings#4807

Open
jesmrec wants to merge 4 commits intomasterfrom
feature/add_action_check_source_strings
Open

[FEATURE REQUEST] Add action to check source strings#4807
jesmrec wants to merge 4 commits intomasterfrom
feature/add_action_check_source_strings

Conversation

@jesmrec
Copy link
Collaborator

@jesmrec jesmrec commented Mar 16, 2026

Two actions were addressed here:

  • In the strings.xml file, some tiny changes to normalize the file against the validation script: remove some useless line breaks, and replace " for the escaped \"

  • The action will run a script to check if every string in strings.xml contains any forbidden character (non-escaped). The list of forbidden characters is set inside the script (i expect the list to be very static). The util xmlstarlet will be used to parse the xml file and provide every entry one by one by using the process substitution operation

For the moment, the check will not be mandatory. Let's give a try, and, if it works fine, we could become mandatory

Related Issues

App:

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

@jesmrec jesmrec self-assigned this Mar 16, 2026
@jesmrec jesmrec added this to the 4.8 - Current milestone Mar 16, 2026
@jesmrec jesmrec force-pushed the feature/add_action_check_source_strings branch 4 times, most recently from 7689a00 to 4bc6ef4 Compare March 16, 2026 11:10
@jesmrec jesmrec marked this pull request as ready for review March 16, 2026 11:52
@jesmrec jesmrec force-pushed the feature/add_action_check_source_strings branch 2 times, most recently from 1fa16c4 to cf3a224 Compare March 17, 2026 13:11
@jesmrec jesmrec requested a review from joragua March 17, 2026 15:34
@joragua joragua changed the title [FEATURE] Add action to check source strings [FEATURE REQUEST] Add action to check source strings Mar 18, 2026
Copy link
Collaborator

@joragua joragua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments here, @jesmrec! 🙌🏻

Comment on lines +1 to +5
Enhancement: GitHub action to validate source strings

A new GitHub action has been added to validate the source strings and prevent errors in translation ci system from the first step

https://github.com/owncloud/android/pull/4807
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, I'd write the calens like this (juts to align all entries for workflows) 😄

A similar example: #4759

Suggested change
Enhancement: GitHub action to validate source strings
A new GitHub action has been added to validate the source strings and prevent errors in translation ci system from the first step
https://github.com/owncloud/android/pull/4807
Enhancement: Workflow to validate source strings
A new workflow has been added to validate the source strings and prevent errors in translation CI system from the first step
https://github.com/owncloud/android/pull/4807

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

<string name="create_space_dialog_empty_error">El nombre del space no puede estar vacío</string>
<string name="create_space_dialog_length_error">El nombre del space no puede exceder 255 caracteres</string>
<string name="create_space_dialog_characters_error">Caracteres prohibidos: / \\ . : \? * &amp;quot; \' &gt; &lt; |</string>
<string name="create_space_dialog_characters_error">Caracteres prohibidos: / \\ . : \? * \" \' &gt; &lt; |</string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are more string files with &amp;quot; and $quot; values:

values-de-rDE/strings.xml
values-de/strings.xml

They should be replaced, right? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, they seem to be added after the first task in the PR. Fixing...

<string name="auth_unsupported_auth_method">The server does not support this authentication method</string>
<string name="auth_fail_get_user_name">Your server is not returning a correct user ID. Please contact an administrator.
</string>
<string name="auth_fail_get_user_name">Your server is not returning a correct user ID. Please contact an administrator.</string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same for indentation. I think there are more string files that need a refactor

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, i put the focus in the strings.xml because is the only file that the workflow checks. Would it be a good idea to check all the translations file to indent or remove line breaks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number of involved strings is very high... it may not woth

@jesmrec jesmrec force-pushed the feature/add_action_check_source_strings branch from a499e9f to 4bf06c0 Compare March 18, 2026 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants