-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add exporter desc to enum analog to import #3606
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
Conversation
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.
I guess "never change a running system" would have been a good advice before we started the whole export refactoring adventure. 😄
The PR looks good and I only have a small suggestion to further simplify the code a bit.
exporters.add(new TemplateExporter("OpenOffice/LibreOffice CSV", "oocsv", "openoffice-csv", "openoffice", FileType.CSV, layoutPreferences, savePreferences)); | ||
exporters.add(new TemplateExporter("RIS", "ris", "ris", "ris", FileType.RIS, layoutPreferences, savePreferences).withEncoding(StandardCharsets.UTF_8)); | ||
exporters.add(new TemplateExporter("MIS Quarterly", "misq", "misq", "misq", FileType.RTF, layoutPreferences, savePreferences)); | ||
exporters.add(new TemplateExporter(FileType.HTML.getDescription(), "html", "html", null, FileType.HTML, layoutPreferences, savePreferences)); |
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.
It appears that now the description is always the description of the file type, right? Then it makes sense to create a new constructor of TemplateExporter
that automatically sets the description.
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.
I thought so, too but on the other hand we have the constructor needed for the custom exporter, so in the end it's the same
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.
but at least we could add a new constructor, so that this list gets more readable, right?
* 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) ...
Fixes #3605
Follow up from #3576
gradle localizationUpdate
?