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

.properties files syncronisation #760

Closed
R3dst0ne opened this issue Jul 2, 2021 · 18 comments
Closed

.properties files syncronisation #760

R3dst0ne opened this issue Jul 2, 2021 · 18 comments
Assignees
Labels
enhancement pri std Standard importance

Comments

@R3dst0ne
Copy link
Contributor

R3dst0ne commented Jul 2, 2021

It might be a good idea to implement some kind of mechanism that enforces the synchronization of all the properties files.
As you can see in #759 they were quite out of sync.

@MarcinOrlowski
Copy link
Member

We should not auto-synchronize because there may be a case where this would be not really desired. But if you'd make your script i.e. compare "base" .properties file with any other and report what is missing (or orphpaned) in the latter and then return non zero RC in such case, then we'd easily make it run as Github Action check

@maehne
Copy link
Member

maehne commented Jul 13, 2021

By default, the English localisation could serve as baseline.

@R3dst0ne
Copy link
Contributor Author

We should not auto-synchronize because there may be a case where this would be not really desired.

I can't think of any situation where we want them to be out of sync. Can you give me an example?

@MarcinOrlowski
Copy link
Member

MarcinOrlowski commented Jul 13, 2021

No, not really, but I see no benefit for automated sync either (we do not automatically fix any issues linters find, even if that'd be doable). The lint can check if these are in sync and if not, flag that. If we include copy-paste steps to the report for developer to use then that's all what we need.

@R3dst0ne
Copy link
Contributor Author

Good point.
As soon as I have some free time I will start working on this.

@MarcinOrlowski
Copy link
Member

Thanks. I think this is two-step effort

  • *.properties file sync checker
  • Github Action

For the first one we just need a way to run the script (existing or new one) that takes at least two files (or just two files if that would help), say A and B and then shows how B differs from A (missing keys, orphaned keys). If any diff exists, aside from printing that out, we return non zero RC. Voila. The rest is GHA task (I think you can easily take most of stuff from .github/workflow/markdown.yml).

@MarcinOrlowski
Copy link
Member

MarcinOrlowski commented Jul 13, 2021

It'd be great if your linter could also deal with the following:

  1. leading spaces (in any case we should not have any in neither A nor B as this is hard to maintain),
  2. if A ends with punctuation mark (i.e. ., : or ?) then if there's B translation, it must follow
  3. No leading space is allowed for punctuation marks nor before \n
  4. Would be great to spot odd escape sequences (or at least \N as I just nailed two cases of that one)
  5. Would be great to have sync percentage rating (X% keys in sync, Y% no missing, Z% no orphaned).

Unfortunately we need to be able to disable check from 2 (via CLI switch) because of Japanese (or non-latin in general) translation.

@maehne
Copy link
Member

maehne commented Jul 13, 2021

I would be wary regarding spacing rules in front of punctuation marks. E.g., in French, a non-breaking space is required in front of colons and question marks. Also, French guillemets for quoting are separated by a non-breaking space from the quoted text.

@MarcinOrlowski
Copy link
Member

MarcinOrlowski commented Jul 13, 2021

a non-breaking space is required in front of colons and question marks

I've seen many french translations and majority did not followed that. So Is it really required? Mind pointing to any reading or so? It'd be really strange (to some degree) to require something like that. It takes more space, so what's the point.

@maehne
Copy link
Member

maehne commented Jul 13, 2021

Look, e.g., here and here for common typographic rules.

@MarcinOrlowski
Copy link
Member

MarcinOrlowski commented Jul 13, 2021

Hm, that's interesting. I just browsed the French Wikipedia and indeed they do waste screen space that way. Well, I just learnt something new today :)

@MarcinOrlowski
Copy link
Member

sync tool needs to understand also the case where "in sync" means that B file has the # ==> key = for corresponging key in A and be told to consider this entry in sync as well as fully translated ones, otherwise we will never have *.properties in sync as we do not have translators on-board 24/7

@R3dst0ne
Copy link
Contributor Author

As mentioned in #761 (comment) this is with the current implementation not possible.
The script, in its current form, was never meant to check for synchronism.

I propose rewriting the script with these checks (sync and content) in mind.
(In one week I will have enough free time to do so.)

@MarcinOrlowski
Copy link
Member

MarcinOrlowski commented Jul 16, 2021

I just released prop-tool utility (Github, PyPi) which implements all the above, so I think this ticket can now be closed.

@R3dst0ne
Copy link
Contributor Author

R3dst0ne commented Jul 16, 2021

As long as it is not implemented, this issue should stay open.

PS. It seems like the link to the GitHub repository is not working.

@MarcinOrlowski
Copy link
Member

Fixed.

@MarcinOrlowski
Copy link
Member

MarcinOrlowski commented Jul 19, 2021

Just a comment I significantly updated prop-tool so now it can do more checks on both base files and the translations. It still probably misses some small things to fully fit the task of guarding properties here but that's should happen in days.

@MarcinOrlowski
Copy link
Member

I am closing this one. As we have the trans-tool based GHA running already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pri std Standard importance
Projects
None yet
Development

No branches or pull requests

3 participants