Fix edition test #6219
Fix edition test #6219
Conversation
Siedlerchr
commented
Mar 30, 2020
Siedlerchr
commented
- Change in CHANGELOG.md described (if applicable)
- Tests created for changes (if applicable)
- Manually tested changed features in running JabRef (always required)
- Screenshots added in PR description (for bigger UI changes)
- Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.
| @@ -54,7 +52,7 @@ public EditionChecker(BibDatabaseContext bibDatabaseContext, boolean allowIntege | |||
| if (!isFirstCharDigit(value) && (!allowIntegerEdition) && !FIRST_LETTER_CAPITALIZED.test(value.trim())) { | |||
| return Optional.of(Localization.lang("should have the first letter capitalized")); | |||
| } else { | |||
| if (!ONLY_NUMERALS.test(value.trim()) && !FIRST_LETTER_CAPITALIZED.test(value.trim())) { | |||
| if (ONLY_NUMERALS.test(value.trim()) && (!allowIntegerEdition) && !FIRST_LETTER_CAPITALIZED.test(value.trim())) { | |||
There was a problem hiding this comment.
The error message says that the first LETTER should be capitalized.
ONLY_NUMBERALS tests for numbers. Numbers are not letters.
Thus, I think, testing for numbers is wrong here.
The old condidion in plain English (which is IMHO right):
In case the edition contains some letters, check that the first letter is capitalized.
The new condidion reads:
In case the text consists of numbers only, then check if nubmers are DISALLOWED.
(In other words: If there are numbers and they are disabled)
Then check whether the first letter is capitalized (side comment: A number is never capitalized (according to the regex at FIRST_LETTER_CAPITALIZED))
I think, the new condidion does not make sense at it mixes numbers and letters.
There was a problem hiding this comment.
Does the code test still work if you revert the change?
There was a problem hiding this comment.
The test obviously didn't work. that's why I had to introduce this change and it's due to the short circuit operation.
- We are in bibtex mode and only have a number (2)
- isFirstCharDigit return true= > Short circuit to else branch
- Only numerals checks if we have one or more numbers => true
3.1 Do we have=> no => return error message
3.2 Do we allow numbers? yes => Is the first one capitalized? Obviously not, cause it's a number^^ (okay, that can be removed
Those were the failing tests:
assertWrong(withMode(createContext(StandardField.EDITION, "2"), BibDatabaseMode.BIBTEX));
assertCorrect(withMode(createContext(StandardField.EDITION, "2"), BibDatabaseMode.BIBTEX), true);
|
Refs #6207 |
|
Fixed the logic and going to merge - to fix master. |
