matthiasblaesing commented on a change in pull request #2169: URL: https://github.com/apache/netbeans/pull/2169#discussion_r491724179
########## File path: ide/db.dataview/src/org/netbeans/modules/db/dataview/output/dataexport/DataViewTableDataExportFileChooser.java ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.db.dataview.output.dataexport; + +import java.io.File; +import java.util.Arrays; +import java.util.List; +import javax.swing.JFileChooser; +import javax.swing.JOptionPane; +import javax.swing.JTable; +import javax.swing.filechooser.FileFilter; +import org.netbeans.api.progress.BaseProgressUtils; +import org.openide.util.NbBundle; + +/** + * + * @author Periklis Ntanasis <[email protected]> + */ [email protected]({ + "LBL_FILE_CHOOSER=Export Table Data", + "LBL_OVEWRITE_DIALOG=Confirm Data Export", + "MSG_OVEWRITE_DIALOG=File already exists.\nDo you want to overwrite it?", + "MSG_EXPORT_DATA=Export Data..." +}) +public class DataViewTableDataExportFileChooser { + + private static final List<DataExporter> EXPORTERS = Arrays.asList( + new CSVDataExporter(), + new TSVDataExporter(), + new XLSXDataExporter() + ); + + private static File defaultDirectory; + + public static void extractAsFile(final JTable table) { + final JFileChooser fc = initializeFileChooser(); + int returnVal = fc.showDialog(null, Bundle.LBL_FILE_CHOOSER()); + switch (returnVal) { + case JFileChooser.APPROVE_OPTION: + FileFilter filter = fc.getFileFilter(); + DataExporter selectedExporter = EXPORTERS.stream() + .filter(exporter -> exporter.getFileFilter() == filter) + .findAny().orElseThrow(() -> new AssertionError("No matching file exporter filter found.")); + final File file = checkFileExtension(fc.getSelectedFile(), selectedExporter); + if (checkFile(file)) { + final String[] columnNames = DataExportUtils.getColumnNames(table); + final Object[][] content = DataExportUtils.getTableContents(table); + BaseProgressUtils.showProgressDialogAndRun( + () -> selectedExporter.exportData( + columnNames, + content, + file), + Bundle.MSG_EXPORT_DATA()); + } + break; + } + } + + private static boolean checkFile(File file) { + if (file.exists()) { + int a = JOptionPane.showConfirmDialog( + null, + Bundle.LBL_OVEWRITE_DIALOG(), + Bundle.MSG_OVEWRITE_DIALOG(), + JOptionPane.YES_NO_OPTION); + return a == JOptionPane.YES_OPTION; + } + return true; + } + + private static File checkFileExtension(File file, DataExporter exporter) { + if (!exporter.handlesFileFormat(file)) { + return new File(file.getAbsolutePath() + "." + exporter.getDefaultFileExtension()); + } + return file; + } + + private static JFileChooser initializeFileChooser() { + final JFileChooser fc = new JFileChooser(); + fc.setAcceptAllFileFilterUsed(false); + EXPORTERS.forEach(exporter -> fc.addChoosableFileFilter(exporter.getFileFilter())); + if (defaultDirectory == null) { + defaultDirectory = fc.getCurrentDirectory(); + } Review comment: > Also, I have implemented the suggestion about storing and using the current directory and also explicitly setting the default file filter. To be honest if I understand correctly the default behavior should also be constant between invocations and always return the user's "default" directory. I guess that this could change in theory while the application is still running but it seems a bit extreme scenario. Anyway, I have made the change and any other suggestion is mote than welcome. Lines 98-100 are unnecessary: https://docs.oracle.com/javase/7/docs/api/javax/swing/JFileChooser.html#setCurrentDirectory(java.io.File) What I meant is, that you should modify `extractAsFile` and there the switch case for `JFileChooser.APPROVE_OPTION`. If the user uses that, query the the current directory and store that. The file chooser on the second call will start in the directory the user selected last. ########## File path: ide/db.dataview/src/org/netbeans/modules/db/dataview/output/dataexport/DataViewTableDataExportFileChooser.java ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.modules.db.dataview.output.dataexport; + +import java.io.File; +import java.util.Arrays; +import java.util.List; +import javax.swing.JFileChooser; +import javax.swing.JOptionPane; +import javax.swing.JTable; +import javax.swing.filechooser.FileFilter; +import org.netbeans.api.progress.BaseProgressUtils; +import org.openide.util.NbBundle; + +/** + * + * @author Periklis Ntanasis <[email protected]> + */ [email protected]({ + "LBL_FILE_CHOOSER=Export Table Data", + "LBL_OVEWRITE_DIALOG=Confirm Data Export", + "MSG_OVEWRITE_DIALOG=File already exists.\nDo you want to overwrite it?", + "MSG_EXPORT_DATA=Export Data..." +}) +public class DataViewTableDataExportFileChooser { + + private static final List<DataExporter> EXPORTERS = Arrays.asList( + new CSVDataExporter(), + new TSVDataExporter(), + new XLSXDataExporter() + ); + + public static void extractAsFile(final JTable table) { + final JFileChooser fc = new JFileChooser(); + fc.setAcceptAllFileFilterUsed(false); + EXPORTERS.forEach(exporter -> fc.addChoosableFileFilter(exporter.getFileFilter())); + int returnVal = fc.showDialog(null, Bundle.LBL_FILE_CHOOSER()); + switch (returnVal) { + case JFileChooser.APPROVE_OPTION: + FileFilter filter = fc.getFileFilter(); Review comment: > Regarding a previous comment about accessing the `JTable` and potentially blocking the `EDT` I am afraid that I don't have any better suggestion without making significant changes to the existing code or this pull request. Some alternatives I can think are: > > 1. Store the query result somewhere else and access it from there. This requires modifying the existing code that performs the query. > > 2. Change approach and instead of exporting to a file whatever the `JTable` displays, execute the query again and export the outcome of the query. I personally like the behavior as introduced by this pull request better so I am against this approach. > > > What do you think? I had another look at db.dataview. I think your extraction code would be easier, if it would work directly on the `DataViewTableUIModel`. That class (or more precise its superclass) already has the necessary accessors: `getData()` and `getColumnName`. That prevents reinventing the wheel. Exporting in this case means exporting the visible data - given that it is only logical to reuse the already queried data. We don't gain much from doing a requery. Storing the query result twice is IMHO a no-go as it will double the necessary heap and we are already suffering from OOMEs. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
