ExportFormats: Use FileExtension enum#3565
Conversation
tobiasdiez
left a comment
There was a problem hiding this comment.
I'm glad you were able to solve your compile problems! Thanks for your contribution. The current code still needs a bit of work before we can merge it. See below for further details.
| ExportFileFilter eff = (ExportFileFilter) ff; | ||
| String path = file.getPath(); | ||
| if (!path.endsWith(eff.getExtension())) { | ||
| if (!path.endsWith(eff.getExtension().toString())) { |
There was a problem hiding this comment.
This will not work: the FileExtensions.toString method will return the default java string serialization and not the extension stored internally. You should use FileExtensions.getExtensionsAsList() instead.
| return true; | ||
| } else { | ||
| return file.getPath().toLowerCase(Locale.ROOT).endsWith(extension); | ||
| return file.getPath().toLowerCase(Locale.ROOT).endsWith(extension.toString()); |
| public ExportFormat(String displayName, String consoleName, String lfFileName, String directory, String extension, | ||
| LayoutFormatterPreferences layoutPreferences, SavePreferences savePreferences) { | ||
| this(displayName, consoleName, lfFileName, directory, extension); | ||
| this(displayName, consoleName, lfFileName, directory, FileExtensions.AUX); |
There was a problem hiding this comment.
Why did you use ̀ AUXhere instead of the providedextension`?
| XML(Localization.lang("%0 file", "XML"), "xml"), | ||
| ZIP(Localization.lang("%0 file", "ZIP"), "zip"); | ||
| ZIP(Localization.lang("%0 file", "ZIP"), "zip"), | ||
| ODS(Localization.lang("%0 file", "ZIP"), "ods"), |
There was a problem hiding this comment.
Please correct the description for ods and sxc.
| } | ||
|
|
||
| public static FileExtensions getFileExtension(String consoleName) { | ||
| ExportFormat exportFormat = (ExportFormat) EXPORT_FORMATS.get(consoleName); |
There was a problem hiding this comment.
What happens if no ExportFormat is not found for that console Name?
Then the exportFormat.getExtension will throw an NPE
Siedlerchr
left a comment
There was a problem hiding this comment.
I think you need to add a default/check for null in that case
LinusDietz
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
* upstream/master: (121 commits) Fix fetcher tests (#3583) Convert entry preview panel to JavaFX (#3574) Remove dependency for JUnit4 (#3580) Update tracis cache config as recommned by the docs (#3575) fix typo Show development information Release v4.1 Add new authors to mailMap (#3567) Fix build Really fix changelog Fix changelog Minor changes Disable the autocompletion as default (#3569) Add update exception for applicationinsights 2.0.0-BETA (#3571) Install4j jres 152 (#3570) Fix grammar mistake Downgrade applicationinsights. Fixes #3561 Adapt scripts/prepare-install4j.sh to JRE1.8.0_152 (#3568) ExportFormats: Use FileExtension enum (#3565) Update guava from 23.5-jre -> 23.6-jre ... # Conflicts: # build.gradle
gradle localizationUpdate?