-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add support for transliterated citation keys #13893
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
base: main
Are you sure you want to change the base?
Conversation
I expect that I chose some wrong naming or placement |
jablib/src/main/java/org/jabref/logic/bibtex/BibtexStandard.java
Outdated
Show resolved
Hide resolved
@ParameterizedTest(name = "string={0}, expected={1}") | ||
@CsvSource({ | ||
"Hello, Hello", // English | ||
"Grüße, Grusse", // German |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be Gruesse with ue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, yeah, but icu4j has other opinion 😅
I will look if there is any option.
But maybe this is the most "general transliteration", because ü, I think, is present not only in German
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dunno, would be okay for me ... I think we cannot get perfect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This triggered that we still use an old patched version of icu4j. 72.0.1,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I can resovle this?
Sorry, forgot a bit about this PR |
It would be nice to have this finished, seems like you just need to adjust the mocks/options in the test? |
} | ||
|
||
@VisibleForTesting | ||
public static CitationKeyPatternPreferences getInstanceForTesting() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to a test util class. This does not belong here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In package testutils?
jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java
Outdated
Show resolved
Hide resolved
on my way
…On Mon, Sep 22, 2025 at 2:58 PM Christoph ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java
<#13893 (comment)>:
> @@ -649,6 +650,7 @@ public JabRefCliPreferences() {
defaults.put(USE_OWNER, Boolean.FALSE);
defaults.put(OVERWRITE_OWNER, Boolean.FALSE);
+ defaults.put(TRANSLITERATE_FIELDS, Boolean.TRUE);
I would make this false by default
—
Reply to this email directly, view it on GitHub
<#13893 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADD5VEUXPUISO77QI3AT4E33T7P7BAVCNFSM6AAAAACGN3KAJ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTENJSGI2DENRZGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
jablib/src/main/java/org/jabref/logic/citationkeypattern/CitationKeyPatternPreferences.java
Show resolved
Hide resolved
return shouldTransliterateFields.get(); | ||
} | ||
|
||
public BooleanProperty shouldTransliterateFieldsProperty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method should return an Optional instead of a BooleanProperty to avoid returning null and to follow modern Java best practices.
jablib/src/main/java/org/jabref/logic/citationkeypattern/CitationKeyPatternPreferences.java
Show resolved
Hide resolved
jablib/src/main/java/org/jabref/logic/citationkeypattern/CitationKeyGenerator.java
Show resolved
Hide resolved
} | ||
|
||
private static String buildTransliteratorConfig() { | ||
StringBuilder pattern = new StringBuilder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using StringBuilder for simple string concatenation can be replaced with StringJoiner for better readability and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really?
JUnit tests of You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide. |
Closes #11377 and #9605.
Added:
Result:
Steps to test
Results:
Mandatory checks
CHANGELOG.md
in a way that is understandable for the average user (if change is visible to the user)