Catch NumberFormatException if context can't be parsed in groups #2488
Conversation
tobiasdiez
added
the
status: ready-for-review
There was a problem hiding this comment.
I have tested this locally and I get the warning when trying to open the bib file in question. So this should be fine. The code looks fine to me, except for one specific thing.
Since you have developer access, next time please create a branch in the official repo and not in your fork ;-)
Also, this is probably a good opportunity to force you to write tests for random stuff, but I am letting it go.
| return errorMessage; | ||
| } | ||
|
|
||
| public static ParserResult getNullResult() { |
There was a problem hiding this comment.
This smells a lot like singleton. There are potential problems in the current implementation, which do probably not manifest, but let us improve this anyway: If you always hand out the same NULL_RESULT, a caller of the method can modify this result (e.g. NULL_RESULT.getDatabase().add(...)), which could theoretically lead to interesting observations at other places.
Long story short: Please always create a new empty ParserResult here. Thus, you can also remove NullResult.
There was a problem hiding this comment.
Good suggestion. I completely inlined the getNullResult method as I don't see any need to encapsulate a constructor call in a new method.
* upstream/master: Fix medline tests...again (#2492) Make sure that unregistered event sources do not stop JabRef from shu… (#2487) Fix #2481: ClassCastException because of wrong cast (#2490) Catch NumberFormatException if context can't be parsed in groups (#2488) Improve CHANGELOG formatting Update guava from 20.0 to 21.0 and mockito-core from 2.5.5 to 2.6.2 Fix aux duplicates (#2480) add update from DOI to the entryeditor sidebar (#2476) Remove obsolete import Add CHANGELOG entry (and one more link) Resolves #2309 JabRef freezes when importing unlinked PDF files into Database Update CHANGELOG.md Fixed bug when assigning refs to groups. emove html code from ACM fetcher before calling parser to prevent junk in bib file (#2473)
Fixes #2477 in the sense that a more detailed error message is displayed.
The functional change is https://github.com/JabRef/jabref/pull/2488/files#diff-48e24aa13e9cd41a20bf4e79f4ad7d6bR129, while the rest is a bit of code cleanup (introduction of helper methods, removal of unused code, change public -> private if applicable).
gradle localizationUpdate?