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.

Sign up for GitHub

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

Fix warning display issue for entering a title made of two words #6054

Merged
merged 10 commits into from
Mar 17, 2020
Merged

Fix warning display issue for entering a title made of two words #6054

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1d4bc67
Fix warning display issue for entering a title made of two sentences
eetian Mar 3, 2020
18fe0d4
Made changes as requested
eetian Mar 4, 2020
331700b
Add test cases for this issue in the exsiting test file and made all …
eetian Mar 7, 2020
3b9c091
Merge branch 'master' into fix-for-issue-5832
eetian Mar 7, 2020
4fa903a
Add test cases for titles having delimiters in TitleCheckerTest and m…
eetian Mar 13, 2020
9133caa
Add test cases for titles having delimiters in TitleCheckerTest and m…
eetian Mar 13, 2020
72b1434
Delete my old bullet point in CHANGELOG.md
eetian Mar 13, 2020
9bc8a6c
Fix the checkstyle issues
eetian Mar 14, 2020
04284e8
Delete one extra empty line in IntegrityCheckTest.java
eetian Mar 14, 2020
7d95ba7
Fix PR Quality Review issues
eetian Mar 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where the group and the link column were not updated after changing the entry in the main table. [#5985](https://github.com/JabRef/jabref/issues/5985)
- We fixed an issue where reordering the groups was not possible after inserting an article. [#6008](https://github.com/JabRef/jabref/issues/6008)
- We fixed an issue where citation styles except the default "Preview" could not be used. [#56220](https://github.com/JabRef/jabref/issues/5622)
- We fixed an issue where warning is displayed when title is made of two sentences. [#5832](https://github.com/JabRef/jabref/issues/5832)
eetian marked this conversation as resolved.
Show resolved Hide resolved
- We fixed an issue where an exception was thrown when adding a save action without a selected formatter in the library properties [#6069](https://github.com/JabRef/jabref/issues/6069)

### Removed
Expand Down
28 changes: 17 additions & 11 deletions src/main/java/org/jabref/logic/integrity/TitleChecker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
public class TitleChecker implements ValueChecker {

private static final Pattern INSIDE_CURLY_BRAKETS = Pattern.compile("\\{[^}\\{]*\\}");
private static final String DELIMITERS = "(\\.|\\!|\\?|\\;|\\:)";
eetian marked this conversation as resolved.
Show resolved Hide resolved
private static final Predicate<String> HAS_CAPITAL_LETTERS = Pattern.compile("[\\p{Lu}\\p{Lt}]").asPredicate();

private final BibDatabaseContext databaseContext;
Expand All @@ -22,10 +23,13 @@ public TitleChecker(BibDatabaseContext databaseContext) {

/**
* Algorithm:
* - remove trailing whitespaces
* - ignore first letter as this can always be written in caps
* - remove everything that is in brackets
* - check if at least one capital letter is in the title
* - split the title into sub titles based on the delimiters
* (defined in the local variable DELIMITERS, currently . ! ? ; :)
* - for each sub title:
* - remove trailing whitespaces
* - ignore first letter as this can always be written in caps
* - check if at least one capital letter is in the sub title
*/
@Override
public Optional<String> checkValue(String value) {
Expand All @@ -37,9 +41,7 @@ public Optional<String> checkValue(String value) {
return Optional.empty();
}

String valueTrimmed = value.trim();
String valueIgnoringFirstLetter = valueTrimmed.startsWith("{") ? valueTrimmed : valueTrimmed.substring(1);
String valueOnlySpacesWithinCurlyBraces = valueIgnoringFirstLetter;
String valueOnlySpacesWithinCurlyBraces = value;
while (true) {
Matcher matcher = INSIDE_CURLY_BRAKETS.matcher(valueOnlySpacesWithinCurlyBraces);
if (!matcher.find()) {
Expand All @@ -48,11 +50,15 @@ public Optional<String> checkValue(String value) {
valueOnlySpacesWithinCurlyBraces = matcher.replaceAll("");
}

boolean hasCapitalLettersThatBibtexWillConvertToSmallerOnes = HAS_CAPITAL_LETTERS
.test(valueOnlySpacesWithinCurlyBraces);

if (hasCapitalLettersThatBibtexWillConvertToSmallerOnes) {
return Optional.of(Localization.lang("capital letters are not masked using curly brackets {}"));
String[] splitTitle = valueOnlySpacesWithinCurlyBraces.split(DELIMITERS);
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 {}"));
}
}
}

return Optional.empty();
Expand Down
24 changes: 24 additions & 0 deletions src/test/java/org/jabref/logic/integrity/IntegrityCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,17 @@ void testTitleChecks() {
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a {Title}"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "{C}urrent {C}hronicle"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "{A Model-Driven Approach for Monitoring {ebBP} BusinessTransactions}"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: This is a sub title 2"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: this is a sub title 2"), BibDatabaseMode.BIBTEX));
assertWrong(withMode(createContext(StandardField.TITLE, "This is a sub title 1: This is A sub title 2"), BibDatabaseMode.BIBTEX));
assertWrong(withMode(createContext(StandardField.TITLE, "This is a sub title 1: this is A sub title 2"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: This is {A} sub title 2"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: this is {A} sub title 2"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1...This is a sub title 2"), BibDatabaseMode.BIBTEX));
assertWrong(withMode(createContext(StandardField.TITLE, "This is a sub title 1... this is a sub Title 2"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is; A sub title 1.... This is a sub title 2"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This!is!!A!Title??"), BibDatabaseMode.BIBTEX));
assertWrong(withMode(createContext(StandardField.TITLE, "This!is!!A!TitlE??"), BibDatabaseMode.BIBTEX));

assertCorrect(withMode(createContext(StandardField.TITLE, "This is a title"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a Title"), BibDatabaseMode.BIBLATEX));
Expand All @@ -194,6 +205,19 @@ void testTitleChecks() {
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a {Title}"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "{C}urrent {C}hronicle"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "{A Model-Driven Approach for Monitoring {ebBP} BusinessTransactions}"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: This is a sub title 2"), BibDatabaseMode.BIBLATEX));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is good to have tests here. These are more a end-to-end-test. Would you mind to add separate tests to TitleCheckerTest.java. In IntelliJ, you can create this file using Ctrl+Shift+T (and pressing Enter in the dialog). It would really help to have the tests for the TitleChecker in a dedicated class, which helps to focus on the implementation. -- This should be very easy to do, but will help future work on the title checker (maybe in one, two years) to ensure that the functionality is still correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your explanation! As suggested, I have added separate tests for titles having delimiters in the new file. Those method names may not be perfect. If there is a better way to rename it, please let me know.

assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: this is a sub title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: This is A sub title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: this is A sub title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: This is {A} sub title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1: this is {A} sub title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1...This is a sub title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is a sub title 1... this is a sub Title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This is; A sub title 1.... This is a sub title 2"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This!is!!A!Title??"), BibDatabaseMode.BIBLATEX));
assertCorrect(withMode(createContext(StandardField.TITLE, "This!is!!A!TitlE??"), BibDatabaseMode.BIBLATEX));


}

@Test
Expand Down