Fix syncLangPy script #3743

Brainsucker92 · 2018-02-19T11:12:28Z

As already mentioned in my comment here: #3573 (comment)
there was a logical issue hidden within the merging process when synchronizing the files. I fixed this issue and added/changed a couple of other things.

Change in CHANGELOG.md described
Tests created for changes
Screenshots added (for bigger UI changes)
Manually tested changed features ~~in running JabRef~~
Check documentation status (Issue created for outdated help page at help.jabref.org?)

Siedlerchr · 2018-02-19T11:43:50Z

Thanks a lot!

koppor · 2018-02-19T14:38:03Z

Escapings

Look at this:

English file:

Executing\ command\ \"%0\"...=Executing command \"%0\"...
The\ marked\ area\ does\ not\ contain\ any\ legible\ text!=The marked area does not contain any legible text!

I think, one has to rewrite the English file to NOT escape ! and ".

Empty Translations

Empty strings must not be added, because of the new Crowdin workflow.

Alternative: Add English as default (and not the empty ones)

LinusDietz · 2018-02-19T16:06:34Z

Does this run with jython?

Brainsucker92 · 2018-02-19T20:19:53Z

All of the code related to python 3 is within a try block. This keeps it's compatibility to python2, so it should be no problem to run it with jython.

By the way there is already a repository for a jython version which is compatible to python 3.5 here:
https://github.com/jython/jython3
But since it's not a stable version at all, I wouldn't recommend switching to jython3.

Brainsucker92 · 2018-02-19T22:01:34Z

@koppor In order to fix the issue with the escapings, I think I have to rework the logging output first. The output is currently not helpful at all, which is just printing a lot of unreadable spam into the console. Once this is fixed it will be much easier to see which keys are causing some trouble during the merging process.

Brainsucker92 · 2018-02-20T20:06:15Z

Quick update:
Today I've been working on the command-line interface using the argparse library. This took me way longer than I expected. Next step will be to rework the logging output, as mentioned before.

Brainsucker92 · 2018-02-21T11:20:59Z

@koppor I think the script is now good to go. Due to my changes in the logging functionality, I found out all malformed strings will be replaced by the correct ones but they have to be translated again, unless the formatting is corrected manually (I think there are "only" 5 malformed keys in the german file, which should be done quickly).

The command-line options are still working exactly the same as before but I also added a new option called print. This option allows to print specific status information for one particular file.

Examples:
syncLang.py print missing -file JabRef_de.properties Prints missing keys of the JabRef_de.properties file
syncLang.py print obsolete -file JabRef_de.properties Prints obsolete keys of the JabRef_de.properties file
syncLang.py print duplicates -file JabRef_de.properties Prints duplicate keys of the JabRef_de.properties file

Would you mind testing the latest version and let me know if you still want to have any changes?

Siedlerchr · 2018-02-21T12:40:44Z

That comes right in time for checking the localization on the maintable beta branch!

koppor · 2018-02-21T22:05:41Z

Would it be possible that print missing can also go through a complete directory and check all files at once? As default, it should use the directory src/main/resources/l10n so that print missing works without any additional options.
It should output if nothing is found -- oh, it does not output if there was something. The status output missing keys

python scripts\syncLang.py print missing -f src\main\resources\l10n\JabRef_de.properties
INFO:root:Unable to use PathFinder class.

Otherwise, it looks good to me. Have to change the Crowdin config then. @JabRef/developers JabRef will now contain values (the english text!) for each translation in the non-English files. This is supported by Crowdin.

Brainsucker92 · 2018-02-22T12:08:22Z

Ok, I changed the print command. The file argument is no longer required for all subcommands (missing, obsolete, duplicates).

You can now use syncLang.py print missing. But keep in mind, this might be a little spammy

tobiasdiez · 2018-02-22T13:24:33Z

@koppor I have the vague memory that we already discussed the problematic of having English as the default value for the other language and decided against this option because it would make the established way of editing the language files directly more difficult. Not sure how relevant this still is because every translation contribution now seems to come through crowdin.

koppor · 2018-02-22T17:02:42Z

@tobiasdiez I think, it was only because we could not recognize easily with the script that the keys have been translated. For instance, following line

OK = OK

Is this translated or not? (In German yes, in Chinese not.)

IMHO this is the edge case. With the change that the English values come into the other language files we enable the guys NOT working with Crowdin to be able to translate. For instance, @josephace135 wants to use GitHub to get rewarded via https://utopian.io.

koppor · 2018-02-23T06:20:42Z

@Brainsucker92 Is it easily possible to add test cases for Python?

It would be really great to test following:

Translated:

Action=Aktion
%0\ contains\ the\ regular\ expression\ <b>%1</b>=%0 enthält den regulären Ausdruck 
<b>%1</b>
The\ marked\ area\ does\ not\ contain\ any\ legible\ text\!=Der markierte Bereich enthält keinen lesbaren Text\!
Generating\ BibTeX\ key\ for=Erzeuge BibTeX-Key für
Hint\:\ To\ search\ specific\ fields\ only,\ enter\ for\ example\:<p><tt>author\=smith\ and\ title\=electrical</tt>=Hinweis\: Um ausschließlich bestimmte Felder zu durchsuchen, geben Sie z.B. ein\:<p><tt>author\=smith and title\=electrical</tt>

Not translated:

Action=Action
%0\ contains\ the\ regular\ expression\ <b>%1</b>=%0 contains the regular expression <b>%1</b>
The\ marked\ area\ does\ not\ contain\ any\ legible\ text!=The marked area does not contain any legible text!
Generating\ BibTeX\ key\ for=Generating BibTeX key for
Hint\:\ To\ search\ specific\ fields\ only,\ enter\ for\ example\:<p><tt>author\=smith\ and\ title\=electrical</tt>=Hint: To search specific fields only, enter for example:<p><tt>author=smith and title=electrical</tt>

These are the strings, I would look up manually and think whether they work. One can document code by adding test cases. And it would be really great if we had that for our syncLang.py script, too.

Brainsucker92 · 2018-02-23T12:29:16Z

@koppor Yes, those test cases are possible for sure, by using the unittest library.
I might add another python file with sone test cases later.

By the way, I will be at Stuttgart University at 3:15 pm today

koppor · 2018-02-24T10:42:42Z

@Brainsucker92 forget about that test cases.

We keep the non-translated strings removed. Reason: IntelliJ can perfectly deal with it:

koppor · 2018-02-24T10:48:38Z

@Brainsucker92 I am sorry to say, but could you remove the functionality to add missing keys in the other language files? Reporting etc. is OK, but not the addition...

Reason: I find non-existing lines better than existing lines with strings from other languages.

Brainsucker92 · 2018-02-25T17:16:38Z

Sure, I can do that. But i think i'll do some other changes as well. The __update_properties method seems to be way to overloaded by things that should be moved to external methods.

Brainsucker92 · 2018-02-28T19:46:59Z

Sorry for the delay. I was working on another improvement of the script but due to upcoming exams I think I won't be able to complete the improvement any time soon. So I simply uploaded the change you requested.

Siedlerchr · 2018-03-02T14:29:51Z

Hi, I just tried to run your script from gradle under Windows and got (apart for the missing python 3 exception):
I tried to run that on a new empty property file for #3776

Actually it fails already with da_properties

$ ./gradlew localizationUpdateExtended
Task :jmh Last added: null
:localizationUpdateExtendedINFO:root:Unable to use PathFinder class.
Traceback (most recent call last):
  File "E:\workspace\JabRef\jabref\scripts\syncLang.py", line 660, in <module>
    main()
  File "E:\workspace\JabRef\jabref\scripts\syncLang.py", line 656, in main
    parsed_args.func(parsed_args)
  File "E:\workspace\JabRef\jabref\scripts\syncLang.py", line 594, in update_command
    syncer.update_properties()
  File "E:\workspace\JabRef\jabref\scripts\syncLang.py", line 296, in update_properties
    self.__update_jabref_properties()
  File "E:\workspace\JabRef\jabref\scripts\syncLang.py", line 303, in _SyncLang__update_jabref_properties
    self.__update_properties(main_property_file=self.main_jabref_preferences, other_property_files=self.__other_jabref_properties())
  File "E:\workspace\JabRef\jabref\scripts\syncLang.py", line 460, in _SyncLang__update_properties
    if keys[key] != "":
KeyError: u'What\\ is\\ Mr.\\ DLib?'
 FAILED

Brainsucker92 · 2018-03-02T14:45:25Z

Thanks for the hint. Should be fixed now, try again.

Siedlerchr · 2018-03-02T14:55:02Z

I fixed it myway (I switched to my Pyhton 3 installation)

         if key is not None:
                    # Do not write empty keys
                    if keys.get(key) != "" and keys.get(key) is not None:
                        other_lines_to_write.append(Keys.format_key_and_value(key=key, value=keys[key]) + "\n")
                else:

That worked for me. However, no keys are copied to my empty properties. Only newlines and comments.

INFO:root:Processing file 'Menu_tl.properties' with 0 Keys
INFO:root:	added 110 missing keys

Just create a new empty property file:
Menu_tl.properties
JabRef_tl.properties

=> No keys are inserted Or is this expected and managed by crowdin?

https://stackoverflow.com/questions/33727149/dict-object-has-no-attribute-has-key

Brainsucker92 · 2018-03-02T14:57:49Z

Yes, because that's what @koppor requested to change.

could you remove the functionality to add missing keys in the other language files?

That's what I did.

All you have to do to revert this change is to uncomment lines 448 and 450.

Siedlerchr · 2018-03-02T16:04:56Z

Okay: I have an idea. After I found out that missing properties make JabRef crash (Those keys that are used in the fxml files) it would be wise to either auto add these ones. Or you just could add an additional gradle task to add the missing keys.

Brainsucker92 · 2018-03-02T16:18:34Z

To be honest: In my opinion we should seriously consider rewriting the whole script to java, if there is no specific reason not to do this. The more changes are made on the script and the more functionality we add, the more complex the script becomes. And at some point python is no longer a viable language because it's primarily well suited for small and simple code which is slowly but surely overextended. Rewriting into Java would also solve any compatibility problems with py2.x and py3, and also get rid of any jython requirement for the whole JabRef project (if there is no other python code in use).

Siedlerchr · 2018-03-02T17:10:24Z

Yes, that sounds like a valid reasonable thing. I think for the moment we can keep it as is and we should discuss this in the next devcall.

koppor · 2018-03-09T09:13:15Z

@tobiasdiez Is it really true that JabRef crashes due to missing keys? I thought, you wrote a comment at some other place that this is not really the case and this can be fixed. I would like to keep the files clean. Now that utopian allows for having crowdin as platform to get Steem Coins, I would say, we do not need to change our workflow and let the language files be lean and clean. Especially because of the issue OK=OK. Is this translated or not (see #3743 (comment))

koppor · 2018-03-09T09:21:18Z

I tried to collect the options using MADR 2.0.1

Only translated strings in language file

Context and Problem Statement

JabRef has translation files JabRef_it.properties, ...
There are translated and unstranslated strings.
Which ones should be in the translation file?

Decision Drivers

Translators should find new strings to translate easily
New strings to translate should be written into JabRef_en.properties to enable translation by the translators
Crowdin should be kept as translation platform, because 1) it is much easier for the translators than the GitHub workflow and 2) it is free for OSS projects.

Considered Options

Only translated strings in language file
Translated and untranslated strings in language file, have value the untranslated string to indicate untranslated
Translated and untranslated strings in language file, have empty to indicate untranslated

Decision Outcome

Chosen option: "Only translated strings in language file", because comes out best (see below.

Pros and Cons of the Options

Only translated strings in language file

Good, because Crowdin supports it
Bad, because translators need tooling to see untranslated strings
Bad, because issues with FXML (JabRef throws exception when started in another language with missing fxml localization #3796)

Translated and untranslated strings in language file, have value the untranslated string to indicate untranslated

Good, because no issues with FXML
Good, because Crowin supports it
Bad, because untranslated strings cannot be identified easily in latin languages

Translated and untranslated strings in language file, have empty to indicate untranslated

Good, because untranslated strings can be identified easily
Good, because works with FMXL (?)
Bad, because Crowdin does not support it

koppor · 2018-03-09T09:24:27Z

I just merged to move things forward, but the discussion on the options is not finalized.

koppor · 2018-03-09T09:32:07Z

I think, we need the script if Crowdin changes its policy or vanishes from the market. In all other cases, our current workflow should be "good enough"? We can add new English language keys (by following the output of the failing tests). These are then synced to Crowdin. After a translator translated something, it goes into JabRef as pull request.

Nevertheless, it is nice to have and other projects may benefit from it. Thus, a pip package (and thus a separate repo) might be the way to go.

Siedlerchr · 2018-03-09T09:45:51Z

Regarding the FXML issues, @tobiasdiez fixed that in #3820

Stefan added 5 commits February 18, 2018 04:48

Fixed unicode string

ea5d56e

Fixed logical issue within merging process

ba7d87d

Added format_key_and_value method

254449f

Added new line for new properties

b00bfdd

Renamed method and moved Path variables into PathFinder class

7a7f33f

Removed empty line, except statement is now catching ImportErrors

759ea5d

Ignoring empty keys again, new keys are added in english by default.

f575245

Siedlerchr changed the title ~~Fix for issue 3573~~ Fix syncLangPy script Feb 19, 2018

Stefan added 2 commits February 19, 2018 22:19

Removed debug messages

19edb0f

Had to kick out the Path variables of the class again. :/

011501d

Brainsucker92 changed the title ~~Fix syncLangPy script~~ [WIP ]Fix syncLangPy script Feb 19, 2018

Brainsucker92 changed the title ~~[WIP ]Fix syncLangPy script~~ [WIP] Fix syncLangPy script Feb 19, 2018

koppor added the PE1718 label Feb 19, 2018

Stefan added 2 commits February 20, 2018 01:59

Improved method names

e864d2c

Reworked command-line interface

d940de0

Stefan added 5 commits February 21, 2018 03:18

Reworked logging functionality, added new command-line options

c5eecef

small fix on ´getJabRefBaseDirectory´ method

35b7f83

forgot brackets...

ded149b

Removed ´getScriptLocation´ method

1379dad

Added ´BASE_DIRECTORY_NAME´ variable to PathFinder class

e7be7ef

koppor self-assigned this Feb 21, 2018

Missing keys will no longer be written to the properties files

fb3bba0

Fixed missing key issue

432a92b

Siedlerchr added the status: devcall label Mar 2, 2018

koppor merged commit 4c18b83 into JabRef:master Mar 9, 2018

Siedlerchr removed the status: devcall label Apr 20, 2018

Brainsucker92 mentioned this pull request Oct 10, 2019

Sync lang fix #5415

Merged

Brainsucker92 deleted the syncLang_fix branch October 10, 2019 12:03

Brainsucker92 changed the title ~~[WIP] Fix syncLangPy script~~ Fix syncLangPy script Oct 10, 2019

koppor mentioned this pull request Oct 10, 2019

Create 0006-only-translated-strings-in-language-file.md #5420

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix syncLangPy script #3743

Fix syncLangPy script #3743

Brainsucker92 commented Feb 19, 2018 •

edited

Loading

Siedlerchr commented Feb 19, 2018

koppor commented Feb 19, 2018

LinusDietz commented Feb 19, 2018

Brainsucker92 commented Feb 19, 2018

Brainsucker92 commented Feb 19, 2018

Brainsucker92 commented Feb 20, 2018

Brainsucker92 commented Feb 21, 2018

Siedlerchr commented Feb 21, 2018

koppor commented Feb 21, 2018

Brainsucker92 commented Feb 22, 2018

tobiasdiez commented Feb 22, 2018

koppor commented Feb 22, 2018

koppor commented Feb 23, 2018 •

edited

Loading

Brainsucker92 commented Feb 23, 2018

koppor commented Feb 24, 2018

koppor commented Feb 24, 2018

Brainsucker92 commented Feb 25, 2018

Brainsucker92 commented Feb 28, 2018

Siedlerchr commented Mar 2, 2018 •

edited

Loading

Brainsucker92 commented Mar 2, 2018

Siedlerchr commented Mar 2, 2018 •

edited

Loading

Brainsucker92 commented Mar 2, 2018 •

edited

Loading

Siedlerchr commented Mar 2, 2018

Brainsucker92 commented Mar 2, 2018 •

edited

Loading

Siedlerchr commented Mar 2, 2018

koppor commented Mar 9, 2018

koppor commented Mar 9, 2018 •

edited

Loading

koppor commented Mar 9, 2018

koppor commented Mar 9, 2018

Siedlerchr commented Mar 9, 2018

Fix syncLangPy script #3743

Fix syncLangPy script #3743

Conversation

Brainsucker92 commented Feb 19, 2018 • edited Loading

Siedlerchr commented Feb 19, 2018

koppor commented Feb 19, 2018

Escapings

Empty Translations

LinusDietz commented Feb 19, 2018

Brainsucker92 commented Feb 19, 2018

Brainsucker92 commented Feb 19, 2018

Brainsucker92 commented Feb 20, 2018

Brainsucker92 commented Feb 21, 2018

Siedlerchr commented Feb 21, 2018

koppor commented Feb 21, 2018

Brainsucker92 commented Feb 22, 2018

tobiasdiez commented Feb 22, 2018

koppor commented Feb 22, 2018

koppor commented Feb 23, 2018 • edited Loading

Brainsucker92 commented Feb 23, 2018

koppor commented Feb 24, 2018

koppor commented Feb 24, 2018

Brainsucker92 commented Feb 25, 2018

Brainsucker92 commented Feb 28, 2018

Siedlerchr commented Mar 2, 2018 • edited Loading

Brainsucker92 commented Mar 2, 2018

Siedlerchr commented Mar 2, 2018 • edited Loading

Brainsucker92 commented Mar 2, 2018 • edited Loading

Siedlerchr commented Mar 2, 2018

Brainsucker92 commented Mar 2, 2018 • edited Loading

Siedlerchr commented Mar 2, 2018

koppor commented Mar 9, 2018

koppor commented Mar 9, 2018 • edited Loading

Only translated strings in language file

Context and Problem Statement

Decision Drivers

Considered Options

Decision Outcome

Pros and Cons of the Options

Only translated strings in language file

Translated and untranslated strings in language file, have value the untranslated string to indicate untranslated

Translated and untranslated strings in language file, have empty to indicate untranslated

koppor commented Mar 9, 2018

koppor commented Mar 9, 2018

Siedlerchr commented Mar 9, 2018

Brainsucker92 commented Feb 19, 2018 •

edited

Loading

koppor commented Feb 23, 2018 •

edited

Loading

Siedlerchr commented Mar 2, 2018 •

edited

Loading

Siedlerchr commented Mar 2, 2018 •

edited

Loading

Brainsucker92 commented Mar 2, 2018 •

edited

Loading

Brainsucker92 commented Mar 2, 2018 •

edited

Loading

koppor commented Mar 9, 2018 •

edited

Loading