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

Localisation test for invalid Strings #3185

Closed
LinusDietz opened this issue Sep 1, 2017 · 13 comments
Closed

Localisation test for invalid Strings #3185

LinusDietz opened this issue Sep 1, 2017 · 13 comments

Comments

@LinusDietz
Copy link
Member

@mlep suggested this in #3184 (comment).

There should be another localisation consistency check that detects invalid characters like spaces in the localisation strings.

@steph37tours
Copy link
Contributor

Hi, I am a very beginner. I would like to try to help. Could you give more details about this issue to help me to find a solution ? Thank you very much.

@Siedlerchr
Copy link
Member

Hi,
thanks for your interest in JabRef. The problem is about the localization keys and their translations in the l10n properties file, e.g. here for English:
https://github.com/JabRef/jabref/blob/master/src/main/resources/l10n/JabRef_en.properties

A solution would be to add a consistency Test to make sure that there are no whitespaces in the keys and the translations. Whitespaces have to be replaced with underscores.
We have already a Test class with consistency checks:
https://github.com/JabRef/jabref/blob/master/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java

You can simply expand that for a new test case.
Some general information about contribution can be found in our contribution guide https://github.com/JabRef/jabref/blob/master/CONTRIBUTING.md

@LinusDietz
Copy link
Member Author

LinusDietz commented Sep 5, 2017

Hi, @steph37tours!
I guess the first step is to check if there are spaces in the localisation strings.
src/main/resources/l10n contains all localisation files. A possible check should detect any spaces in these files that are not commented out (using the #).

EDIT:

For example

Finished_automatically_setting_external_links.=Selesai_pengaturan_otomatis_tautan_eksternal. is fine

Finished_automatically_setting_external_links.=Selesai_pengaturan_otomatis tautan_eksternal. should not pass your test

Make sure to open your pull request early, we are happy to help you!

@steph37tours
Copy link
Contributor

Thank you for your quick answers ! I look at this and I come back.

@steph37tours
Copy link
Contributor

Firstable, thank you very much for your help !
So I write the beginning of the method. But I'm not sure for instance how to parse all the properties object. And after that, how to replace the spaces by an underscore.
It was too my first pull request.

@LinusDietz
Copy link
Member Author

LinusDietz commented Sep 5, 2017

Well, I think you shouldn't replace the spaces, just let the test fail, so the author will be notified that there are invalid characters in the localization properties file.

Regarding the parsing, just read in all properties as done in the other tests of https://github.com/JabRef/jabref/blob/master/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java and then check if there are spaces in the strings.
Make sure to include a meaningful message to the failing test, so the invalid string can be found!

Good luck, and give us a shout if you need further help!

@steph37tours
Copy link
Contributor

I just push again. I think it's good. Does I have to make a new pull request ?

@steph37tours
Copy link
Contributor

I use properties.replace(" ",""). It was not possible to use replaceAll. I think it's not a trouble as long as the assertEquals compare the String object, with the modified. Just one replace make a difference.

@steph37tours
Copy link
Contributor

I try to test by put a space in properties file, but I didn't have error after launch...

@LinusDietz
Copy link
Member Author

LinusDietz commented Sep 5, 2017

Let's discuss the PR in the PR-Thread.

You can run the tests locally in your IDE or using gradle check.

@aayush2610
Copy link

aayush2610 commented Sep 5, 2017 via email

@LinusDietz
Copy link
Member Author

Hey @aayush2610, Mr. @steph37tours is working on this at #3198, and I'm sure he will resolve this soon.

Feel free to jump into any other issue. There are some labeled with beginner which we think are easier to resolve, even if you are new to JabRef.

@LinusDietz
Copy link
Member Author

fixed in #3198

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants