Skip to content

Add test cases in testCheckLegalKey #9931

Merged
Siedlerchr merged 7 commits into
JabRef:mainfrom
vicluo96:fix-for-issue-9799
Jun 2, 2023
Merged

Add test cases in testCheckLegalKey #9931
Siedlerchr merged 7 commits into
JabRef:mainfrom
vicluo96:fix-for-issue-9799

Conversation

@vicluo96

@vicluo96 vicluo96 commented May 22, 2023

Copy link
Copy Markdown
Contributor

Add three test cases for method testCheckLegalKey in org.jabref.logic.citationkeypattern.CitationKeyGeneratorTest.java
Related to the edge cases in #9799 disscussion
Two of the tests fail because they are edge cases.
Screen Shot 2023-05-21 at 6 59 04 PM
Screen Shot 2023-05-21 at 6 58 47 PM

To solve this, we need to put more characters in UnicodeToReadableCharMap

### Compulsory checks
- [ ] Change in `CHANGELOG.md` described in a way that is understandable for the average user (if applicable)
- [ ] Tests created for changes (if applicable)
- [x] Manually tested changed features in running JabRef (always required)
- [ ] Screenshots added in PR description (for UI changes)
- [ ] [Checked developer's documentation](https://devdocs.jabref.org/): Is the information available and up to date? If not, I outlined it in this pull request.
- [ ] [Checked documentation](https://docs.jabref.org/): Is the information available and up to date? If not, I created an issue at <https://github.com/JabRef/user-documentation/issues> or, even better, I submitted a pull request to the documentation repository.

@vicluo96 vicluo96 changed the title Fix for issue 9799 Fix for issue May 22, 2023
@vicluo96 vicluo96 changed the title Fix for issue Add test cases in testCheckLegalKey May 22, 2023
@ThiloteE ThiloteE added component: citationkey-generator dev: code-quality Issues related to code or architecture decisions labels May 22, 2023
@calixtus

Copy link
Copy Markdown
Member

Thanks for your interest in JabRef and for adding the test case. We would love to have a patch for the CitationKeyGenerator so the test wont fail anymore.

@vicluo96

vicluo96 commented May 27, 2023

Copy link
Copy Markdown
Contributor Author

Sure, I just add a patch to UnicodeToReadableCharMap and all the tests pass in my local machine. However, since we have to manually add all the special characters in UnicodeToReadableCharMap one by one, if a user enter a name that contains characters not in map in the future, this bug will show up again. Maybe we can come up with a clever way to include all the accents' unicode in the future

@calixtus

Copy link
Copy Markdown
Member

Some time ago, @koppor and me were thinking of a replacement for the current Unicode mapping table, but we did not see a systematic in the Unicode table that would avoid mapping each single edge case. But maybe you see a solution? Maybe by checking a range of the index in the Unicode table?

@Siedlerchr

Copy link
Copy Markdown
Member

I would appreciate it if you could try the icu4j transliteration methods:
Article talks about php, but icu is available for java and we already use that library (icu4j)

https://unicode-org.github.io/icu/userguide/transforms/general/
https://unicode-org.github.io/icu-docs/apidoc/dev/icu4j/com/ibm/icu/text/Transliterator.html
https://bartvanraaij.dev/2020-10-17-converting-utf8-strings-to-ascii-using-icu-transliterator/

@Siedlerchr

Copy link
Copy Markdown
Member

I think for the moment we can merge this and create a follow up issue investigating some alternatives

@Siedlerchr Siedlerchr merged commit 9c5be3e into JabRef:main Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: citationkey-generator dev: code-quality Issues related to code or architecture decisions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants