Check Integrity year check added #1906
Conversation
|
Thanks! The tests fails because of missing translation strings. If you run the logic/i10/LocalizationConsistencyTest.java you will get a list of strings which are not in the translation files. Add those to the English one and run the script that the output tells you to. Then, all languages are updated and the test should pass (you may have to refresh the files in your IDE in case it fails again). Also, see: https://github.com/JabRef/jabref/wiki/Code-Howtos#using-localization-correctly If you can add a test to IntegrityCheckTest (or whatever it is called) that really tests for this new feature it would be great. |
|
I hope, it is correct now. Please set label "ready-for-review". Thanks. |
Siedlerchr
added
the
status: ready-for-review
| @@ -16,6 +16,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# | |||
| - [#1496](https://github.com/JabRef/jabref/issues/1496) Keep track of which entry a downloaded file belongs to | |||
| - Made it possible to download multiple entries in one action | |||
| - [#1813](https://github.com/JabRef/jabref/issues/1813) Import/Export preferences dialog default directory set to working directory | |||
| - [#1897](https://github.com/JabRef/jabref/issues/1897) Added last four nonpunctuation characters should be numerals in Check Integrity year | |||
There was a problem hiding this comment.
I would reword it a bit to make it a a bit more clear:
Implemented integrity check for year field: Last four nonpunctuation characters should be numerals
(Put the word "year" in backticks (the french accent grave), just look at other examples in the changelog)
|
Thanks for your work 👍 LGTM! |
| @@ -321,6 +323,12 @@ private BracketChecker(String field) { | |||
| return Collections.singletonList(new IntegrityMessage(Localization.lang("should contain a four digit number"), entry, FieldName.YEAR)); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
We should add a comment to the BibTeX doc here, because this decision on year structure is not intuitive.
|
A probably stupid question: why not just check that the year field consists exactly of 4 digits? |
|
@tobiasdiez Because the offical bibtex spec says so: |
| .asPredicate(); | ||
| private static final String regex = "[](){},.;!?<>%&$]"; |
There was a problem hiding this comment.
Please uppercase constant name and call it something like PUNCTUATION_MARKS. Just calling it regex doesn't really help in understanding what it's for.
|
Got 3 minor comments and then we can merge 👍 |
| assertWrong(createContext("year", "abc")); | ||
| assertWrong(createContext("year", "86")); | ||
| assertWrong(createContext("year", "204")); | ||
| assertWrong(createContext("year", "1986a")); |
There was a problem hiding this comment.
Please add also a test case with one of the regex symbols
|
Thanks 😄 LGTM 👍 |
|
LGTM, too. |
#1897 Added last four nonpunctuation characters should be numerals in Check Integrity year