-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Lookup filetypes in enum set to prevent NPE due to uninitialized expo… #3597
Conversation
…rterFactory Fixes #3596
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for introducing this bug and thanks for the quick fix. The solution you provide works but I think we can push it a bit further:
- the methods
public void addFormat(List<String> s, LayoutFormatterPreferences layoutPreferences, SavePreferences savePreferences) {
private Optional<TemplateExporter> createFormat(List<String> s, LayoutFormatterPreferences layoutPreferences,
can be improved by not using aList<String>
array but just a normal list of three arguments. - The code to determine the
FileType
from the string should already happen here:
jabref/src/main/java/org/jabref/gui/exporter/CustomExportDialog.java
Lines 198 to 207 in d381839
public String extension() { String ext = extension.getText(); if (ext.startsWith(".")) { return ext; } else if (ext.startsWith("*.")) { return ext.substring(1); } else { return '.' + ext; } }
so that the rest of the code usesFileType
and not a string. - the conversion string -> FileType should probably be moved to a static method
parse
inFileType
.
May I ask you to do these further refactoring? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-wise, I have nothing to add to @tobiasdiez comments.
I have also executed the branch locally and JabRef starts up as expected.
Add FileType parse
I refactored the list where possible. This can be further improved if this rather simple dialog is ported to javafx (for example, getting rid of the glazedcel EventListl dependency) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* upstream/master: (40 commits) CleanupStep.MOVE_PDF is not a CleanupPreset anymore Add link and fix casing Explicitly set jacoco version (#3617) Update gradle from 4.4 to 4.4.1 Update bcprov-jdk15on from 1.58 -> 1.59 Fix NPE when changing entries between databases Add exporter desc to enum analog to import (#3606) Create 0001-use-crowdin-for-translations.md [ci skip] Get more Open Source Helpers (#3601) Update dependencies (#3602) Lookup filetypes in enum set to prevent NPE due to uninitialized expo… (#3597) Make it possible to disable autocompletion in the search bar (#3600) New translations JabRef_en.properties (Chinese Simplified) New translations JabRef_en.properties (French) New translations JabRef_en.properties (German) New translations JabRef_en.properties (Indonesian) New translations JabRef_en.properties (Italian) New translations JabRef_en.properties (Japanese) New translations JabRef_en.properties (Russian) New translations JabRef_en.properties (Spanish) ...
…rterFactory
Fixes #3596
This is my initial idea. Maybe we can optimize this
gradle localizationUpdate
?