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

Ignore initial unprotected uppercase in translated titles #10459

Closed
ryan-carpenter opened this issue Oct 7, 2023 · 10 comments · Fixed by #10461
Closed

Ignore initial unprotected uppercase in translated titles #10459

ryan-carpenter opened this issue Oct 7, 2023 · 10 comments · Fixed by #10461
Assignees
Labels
good first issue An issue intended for project-newcomers. Varies in difficulty. type: enhancement

Comments

@ryan-carpenter
Copy link

Some databases store translated titles in the title field, surrounded by square brackets like this.

title = {[Entry with a translated title]}

Because the title phrase begins at the second position of the title string JabRef warns that the initial uppercase letter needs to be protected. To avoid matching this pattern, the regex could include something like [A-Z](?<!^\[.) or (?<^!\[)[A-Z].

image

image

@ryan-carpenter
Copy link
Author

ryan-carpenter commented Oct 7, 2023

private static final Pattern INSIDE_CURLY_BRAKETS = Pattern.compile("\\{[^}\\{]*\\}");

private static final Pattern DELIMITERS = Pattern.compile("\\.|\\!|\\?|\\;|\\:");

private static final Predicate<String> HAS_CAPITAL_LETTERS = Pattern.compile("[\\p{Lu}\\p{Lt}]").asPredicate();

Possibly one of these changes would work in line 15, but I am not sure if the patterns only match if there is white space following the delimiters.
Pattern.compile("\\.|\\!|\\?|\\;|\\:"|\\A\\[);
Pattern.compile("\\.|\\!|\\?|\\;|\\:"|^\\[);

@Siedlerchr Siedlerchr added type: enhancement good first issue An issue intended for project-newcomers. Varies in difficulty. labels Oct 8, 2023
@github-project-automation github-project-automation bot moved this to Normal priority in Features & Enhancements Oct 8, 2023
@Siedlerchr
Copy link
Member

I added this to the project board, I think it's a valid issue.
Additional requirement: Write/Enhance Unit test

@michalfarago
Copy link
Contributor

michalfarago commented Oct 8, 2023

Hello, I'd like to inquire about this issue.

If I've understood it correctly, you are talking about the following code:

for (String subTitle : splitTitle) {
subTitle = subTitle.trim();
if (!subTitle.isEmpty()) {
subTitle = subTitle.substring(1);
if (HAS_CAPITAL_LETTERS.test(subTitle)) {
return Optional.of(Localization.lang("capital letters are not masked using curly brackets {}"));
}
}
}

You'd like to handle this scenario by modifying the pattern and create unit tests. Correct?

@Siedlerchr
Copy link
Member

FIrst create a test case for the behavior.
Then adjust the pattern/code etc to take this into account
The test could also be converted to a parameterized test

@ManaMatrix
Copy link

Hey team I'm new to open-source contributing, I'd love to work on this issue if it is still free to take. Thank you

@michalfarago
Copy link
Contributor

Hello, I've already started working on this. Could you please assign this to me instead?

@ThiloteE
Copy link
Member

ThiloteE commented Oct 9, 2023

@ManaMatrix please check out our other good first issues.

@michalfarago
Copy link
Contributor

Hello, I'd like to double-check the following:

Titles like: title = {Folding kinetics of a polymer [corrigendum]}, are supported and don't raise a warning.

From the issue description, it's clear that the following should also not raise a warning: title = {[Translated title]},

My question is whether the following should raise a warning: title = {[Translated title] Original title},

I've checked the resource folder, but didn't find any entry of that format, so I suspect that it should raise a warning. Let me know if I'm mistaken.

Thank you.

@Siedlerchr
Copy link
Member

@michalfarago Good question, I think it should still raise a warning. It's at least unusual do have both in the same entry
Maybe @ryan-carpenter has a preference here as well?

@github-project-automation github-project-automation bot moved this from Normal priority to Done in Features & Enhancements Oct 19, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Good First Issues Oct 19, 2023
@Siedlerchr
Copy link
Member

@ryan-carpenter The issue is now fixed in the latest development version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue An issue intended for project-newcomers. Varies in difficulty. type: enhancement
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants