-
-
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
Export pdf/linked files #3147
Export pdf/linked files #3147
Changes from 5 commits
9d38d12
00eccc7
50e9716
26a7e1f
ec7c44b
68e1562
d1b53eb
7021371
6b02e9c
edec73d
95474ea
bed0a54
e347bc9
090a4cd
93f72b2
353e9ad
f29066c
aa10731
9e6cffc
b61082c
713ecec
cfa74e5
4c01181
cc30b98
a67cd4e
5d96526
8011e6e
bb1a304
5e1117a
8dabe6d
fc7a2a1
ba35360
88faa7c
658d97e
8f14cc7
07ee6ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,164 @@ | ||||
package org.jabref.gui.actions; | ||||
|
||||
import java.awt.event.ActionEvent; | ||||
import java.io.BufferedWriter; | ||||
import java.io.IOException; | ||||
import java.nio.charset.StandardCharsets; | ||||
import java.nio.file.Files; | ||||
import java.nio.file.Path; | ||||
import java.nio.file.Paths; | ||||
import java.util.List; | ||||
import java.util.Optional; | ||||
import java.util.function.BiFunction; | ||||
|
||||
import javax.swing.AbstractAction; | ||||
|
||||
import javafx.concurrent.Service; | ||||
import javafx.concurrent.Task; | ||||
import javafx.scene.control.Button; | ||||
import javafx.scene.control.ButtonType; | ||||
import javafx.scene.control.DialogPane; | ||||
|
||||
import org.jabref.Globals; | ||||
import org.jabref.JabRefGUI; | ||||
import org.jabref.gui.DialogService; | ||||
import org.jabref.gui.FXDialogService; | ||||
import org.jabref.gui.util.DefaultTaskExecutor; | ||||
import org.jabref.gui.util.DirectoryDialogConfiguration; | ||||
import org.jabref.logic.l10n.Localization; | ||||
import org.jabref.logic.util.OS; | ||||
import org.jabref.logic.util.io.FileUtil; | ||||
import org.jabref.model.database.BibDatabaseContext; | ||||
import org.jabref.model.entry.BibEntry; | ||||
import org.jabref.model.entry.LinkedFile; | ||||
import org.jabref.model.util.FileHelper; | ||||
import org.jabref.model.util.OptionalUtil; | ||||
import org.jabref.preferences.JabRefPreferences; | ||||
|
||||
import org.apache.commons.logging.Log; | ||||
import org.apache.commons.logging.LogFactory; | ||||
import org.controlsfx.dialog.ProgressDialog; | ||||
|
||||
public class ExportLinkedFilesAction extends AbstractAction { | ||||
|
||||
private static final Log LOGGER = LogFactory.getLog(ExportLinkedFilesAction.class); | ||||
private final BiFunction<Path, Path, Path> resolvePathFilename = (path, file) -> { | ||||
return path.resolve(file.getFileName()); | ||||
}; | ||||
private final DialogService ds = new FXDialogService(); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, better name please. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not fixed. |
||||
private long totalFilesCount; | ||||
private BibDatabaseContext databaseContext; | ||||
private Optional<Path> exportPath = Optional.empty(); | ||||
private List<BibEntry> entries; | ||||
|
||||
public ExportLinkedFilesAction() { | ||||
super(Localization.lang("Copy attached files to folder...")); | ||||
} | ||||
|
||||
@Override | ||||
public void actionPerformed(ActionEvent e) { | ||||
|
||||
DirectoryDialogConfiguration dirDialogConfiguration = new DirectoryDialogConfiguration.Builder() | ||||
.withInitialDirectory(Paths.get(Globals.prefs.get(JabRefPreferences.EXPORT_WORKING_DIRECTORY))) | ||||
.build(); | ||||
entries = JabRefGUI.getMainFrame().getCurrentBasePanel().getSelectedEntries(); | ||||
exportPath = DefaultTaskExecutor | ||||
.runInJavaFXThread(() -> ds.showDirectorySelectionDialog(dirDialogConfiguration)); | ||||
|
||||
exportPath.ifPresent(path -> { | ||||
databaseContext = JabRefGUI.getMainFrame().getCurrentBasePanel().getDatabaseContext(); | ||||
totalFilesCount = entries.stream().flatMap(entry -> entry.getFiles().stream()).count(); | ||||
|
||||
Service<Void> exportService = new ExportService(); | ||||
startServiceAndshowProgessDialog(exportService); | ||||
}); | ||||
} | ||||
|
||||
private <V> void startServiceAndshowProgessDialog(Service<V> service) { | ||||
DefaultTaskExecutor.runInJavaFXThread(() -> { | ||||
service.start(); | ||||
|
||||
ProgressDialog progressDialog = new ProgressDialog(service); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please extract the "show progress dialog" part of this method to the |
||||
progressDialog.setOnCloseRequest(evt -> service.cancel()); | ||||
DialogPane dialogPane = progressDialog.getDialogPane(); | ||||
dialogPane.getButtonTypes().add(ButtonType.CANCEL); | ||||
Button cancelButton = (Button) dialogPane.lookupButton(ButtonType.CANCEL); | ||||
cancelButton.setOnAction(evt -> { | ||||
service.cancel(); | ||||
progressDialog.close(); | ||||
}); | ||||
progressDialog.showAndWait(); | ||||
}); | ||||
} | ||||
|
||||
private class ExportService extends Service<Void> { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is rather nitpicky, but what about extracting this into a separate class file in the same package with default visibility? You could also extract parts of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see no reason why I should extract that in a new class. It's just a single Service for one purpose. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One reason is that you could (and should) write tests for the |
||||
|
||||
private static final String LOGFILE = "exportLog.log"; | ||||
|
||||
@Override | ||||
protected Task<Void> createTask() { | ||||
return new Task<Void>() { | ||||
|
||||
int totalFilesCounter; | ||||
int numberSucessful; | ||||
int numberError; | ||||
Optional<Path> newPath; | ||||
|
||||
@Override | ||||
protected Void call() | ||||
throws InterruptedException, IOException { | ||||
updateMessage(Localization.lang("Copying files...")); | ||||
updateProgress(0, totalFilesCount); | ||||
|
||||
try (BufferedWriter bw = Files.newBufferedWriter(exportPath.get().resolve(LOGFILE), StandardCharsets.UTF_8)) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion, a log file is an inconvenient solution for the user. Why don't log all the errors to an internal list and display this list at the end in a new dialog? |
||||
|
||||
for (int i = 0; i < entries.size(); i++) { | ||||
|
||||
List<LinkedFile> files = entries.get(i).getFiles(); | ||||
|
||||
for (int j = 0; j < files.size(); j++) { | ||||
updateMessage(Localization.lang("Copying file %0 of BibEntry %1", Integer.toString(j + 1), Integer.toString(i + 1))); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we never used |
||||
Thread.sleep(500); //TODO: Adjust/leave/any other idea? | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sleep makes the whole progress of copying files really slow. I would remove it even through then the user has no chance to actually read the message for small files (but that is ok in my opinion). |
||||
|
||||
String fileName = files.get(j).getLink(); | ||||
Optional<Path> fileToExport = FileHelper.expandFilenameAsPath(fileName, | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use
|
||||
databaseContext.getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences())); | ||||
|
||||
newPath = OptionalUtil.combine(exportPath, fileToExport, resolvePathFilename); | ||||
|
||||
newPath.ifPresent(newFile -> { | ||||
boolean success = FileUtil.copyFile(fileToExport.get(), newFile, false); | ||||
updateProgress(totalFilesCounter++, totalFilesCount); | ||||
if (success) { | ||||
updateMessage(Localization.lang("Copied file successfully")); | ||||
numberSucessful++; | ||||
try { | ||||
bw.write(Localization.lang("Copied file successfully") + ": " + newFile); | ||||
bw.write(OS.NEWLINE); | ||||
} catch (IOException e) { | ||||
LOGGER.error("error writing log file", e); | ||||
} | ||||
} else { | ||||
updateMessage(Localization.lang("Could not copy file") + ": " + Localization.lang("File exists")); | ||||
numberError++; | ||||
try { | ||||
bw.write(Localization.lang("Could not copy file") + ": " + Localization.lang("File exists") + ":" + newFile); | ||||
bw.write(OS.NEWLINE); | ||||
} catch (IOException e) { | ||||
LOGGER.error("error writing log file", e); | ||||
} | ||||
} | ||||
}); | ||||
} | ||||
} | ||||
updateMessage(Localization.lang("Finished copying")); | ||||
updateMessage(Localization.lang("Copied %0 files of %1 sucessfully to %2", Integer.toString(numberSucessful), Integer.toString(totalFilesCounter), newPath.map(Path::getParent).map(Path::toString).orElse(""))); | ||||
bw.write(Localization.lang("Copied %0 files of %1 sucessfully to %2", Integer.toString(numberSucessful), Integer.toString(totalFilesCounter), newPath.map(Path::getParent).map(Path::toString).orElse(""))); | ||||
return null; | ||||
} | ||||
} | ||||
}; | ||||
} | ||||
} | ||||
|
||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
import org.jabref.gui.IconTheme; | ||
import org.jabref.gui.JabRefFrame; | ||
import org.jabref.gui.actions.Actions; | ||
import org.jabref.gui.actions.ExportLinkedFilesAction; | ||
import org.jabref.gui.filelist.FileListTableModel; | ||
import org.jabref.gui.keyboard.KeyBinding; | ||
import org.jabref.gui.mergeentries.FetchAndMergeEntry; | ||
|
@@ -102,6 +103,7 @@ public RightClickMenu(JabRefFrame frame, BasePanel panel) { | |
|
||
add(new GeneralAction(Actions.SEND_AS_EMAIL, Localization.lang("Send as email"), IconTheme.JabRefIcon.EMAIL.getSmallIcon())); | ||
addSeparator(); | ||
add(new ExportLinkedFilesAction()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you really think that this export functionality is used that often that we should add it to the right click menu? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was requested by @koppor And I think it makes sense to have it in the context menu |
||
|
||
JMenu markSpecific = JabRefFrame.subMenu(Localization.menuTitle("Mark specific color")); | ||
markSpecific.setIcon(IconTheme.JabRefIcon.MARK_ENTRIES.getSmallIcon()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -313,12 +313,6 @@ public void performExport(final BibDatabaseContext databaseContext, final String | |
|
||
} | ||
|
||
@Override | ||
public void performExport(final BibDatabaseContext databaseContext, Path file, final Charset encoding, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please don't delete this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I deleted it on purpose because it is not working at all as I explained in my other PR or earlier. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok... |
||
List<BibEntry> entries) throws Exception { | ||
performExport(databaseContext, file.getFileName().toString(), encoding, entries); | ||
} | ||
|
||
/** | ||
* See if there is a name formatter file bundled with this export format. If so, read | ||
* all the name formatters so they can be used by the filter layouts. | ||
|
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.
Why is there a blank line here?
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.
Oops.