[WIP] Add Text File Export for "Find Unlinked Files" #3979

jssander · 2018-04-25T14:56:07Z

This is to add the feature which allows exporting of the list of files found in "Find Unlinked Files" to a text file.

Closes issue #3341

I'm not sure if it is exactly what you are looking for, but I thought I would give it a shot. Hopefully you will be kind though I welcome constructive feedback of course.

Siedlerchr

Some minor changes, overall good work for your first PR here!

Siedlerchr · 2018-04-25T16:44:39Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+
+        VerifyingWriter verifyingWriter;
+        try {
+            verifyingWriter = new VerifyingWriter(Files.newOutputStream(exportPath.get()), Charset.defaultCharset());


Please use Charset utf8

I think you don't need a Veryfying Writer here, a BufferedWriter should be enough:
And you should use the try with resources , which has the advantage that any stream will be closed automatically, also on error.
Example:

try (BufferedWriter writer = Files.newBufferedWriter(logFile, StandardCharsets.UTF_8, StandardOpenOption.WRITE)) {

Siedlerchr · 2018-04-25T18:41:43Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+
+        CheckableTreeNode root = (CheckableTreeNode) treeModel.getRoot();
+
+        final List<File> fileList = getFileListFromNode(root);


It would be preferable if you change the return type from File to Path of the method, as the Path-Class is part of the java.nio and therefore the more modern way.

This method is used for the importing which I basically copied and then modified to do the exporting.

Siedlerchr · 2018-04-25T18:47:41Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+            return;
+        }
+
+        threadState.set(true);


What is the purpose of this variable/method?

I'm not sure exactly. It is a hold over from the import method which I based the export method on. I guessed that it was needed to start the thread.

Siedlerchr · 2018-04-25T18:50:48Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+
+        threadState.set(true);
+        JabRefExecutorService.INSTANCE.execute(() -> {
+            List<String> errors = new LinkedList<>();


According to the codacy output, the variable errors is never used. And in 90% the cases a normal ArrayList is sufficient, a LinkedList costs more memory. The benefits of a LinkedList seldom outweigh the memory usage in comparsion to ArrayList

I'll remove it.

jssander · 2018-04-25T19:22:18Z

I'm not sure it really even needs the progress bar for the exporting because it is so fast, but I left it in for now since the importing has one. One change I didn't make is changing the list of files to Path type from File type. I left a note about that above. Let me know what you think.

koppor

Some very minor code formatting comments. Would be nice if they were integrated.

koppor · 2018-04-26T13:15:21Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

@@ -527,6 +535,88 @@ public void stateChanged(ChangeEvent e) {
        });
    }

+    /**
+     * This will start the export of all files of all selected nodes in this


"This starts the export"

koppor · 2018-04-26T13:15:29Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+     * invoked.
+     */
+    private void startExport() {
+


Remove empty line.

koppor · 2018-04-26T13:15:46Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+        CheckableTreeNode root = (CheckableTreeNode) treeModel.getRoot();
+
+        final List<File> fileList = getFileListFromNode(root);
+


Please remove empty line

koppor · 2018-04-26T13:15:55Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+    /**
+     */
+    private void exportFinishedHandler() {
+


Please remove empty line

koppor · 2018-04-26T13:16:06Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+        exportFinishedHandler();
+    }
+
+    /**


Please remove empty comment.

koppor · 2018-04-26T13:17:04Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+        setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);
+        frame.getCurrentBasePanel().markBaseChanged();
+    }
+


If you are on it - please remove the method comment 😇 - or add description. The functionality is not exactly clear by the method name.

koppor · 2018-04-26T13:18:49Z

@jssander Please pull before you continue to work on it. I merged master to fix conflicts due to our release today.

tobiasdiez

Thanks for your contribution. The code looks good and I only have a few minor remarks.

tobiasdiez · 2018-04-26T17:34:17Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+
+        Optional<Path> exportPath = DefaultTaskExecutor
+                .runInJavaFXThread(() -> ds.showFileSaveDialog(fileDialogConfiguration));
+


Please add a check if exportPath is present. If not (i.e. the user clicked cancel), then we should return early.

tobiasdiez · 2018-04-26T17:36:17Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

+                for (File file : fileList) {
+                    writer.write(file.toString() + "\n");
+                    counter++;
+                    progressBarImporting.setValue(counter);


As you say, writing to the external file should be very quick (its below <1k lines in almost every case and usual not more then say 50). Thus the progressbar would only flicker a bit and I would hence propose to perform the export silently without it.

tobiasdiez · 2018-04-26T17:37:20Z

src/main/java/org/jabref/gui/FindUnlinkedFilesDialog.java

@@ -609,6 +701,8 @@ private void setupActions() {
         * selected nodes in this dialogs tree view. <br>
         */
        ActionListener actionListenerImportEntrys = e -> startImport();
+        ActionListener actionListenerExportEntrys = e -> startExport();
+        buttonExport.addActionListener(actionListenerExportEntrys);


I think you can simply inline actionListenerExportEntrys (and the other listener before it too, if its not used somewhere else).

jssander · 2018-04-27T16:07:33Z

How do I pull correctly? In IDEA when I go to pull I think I'm pulling from my forked master.

jssander · 2018-05-01T18:48:58Z

I made some changes, but there is a conflict now in the changelog. I'm not sure how to resolve it.

koppor · 2018-05-02T05:57:40Z

@jssander Just keep both lines. I did it for you using the GitHub web ui. Just pull and you are fine at your side, too.

tobiasdiez

Since this PR already as a few positive reviews and the code looks good to me as well, I'll merge now.

Thanks again for your contribution and we are looking forward to your next PR!

jssander · 2018-05-02T15:16:38Z

Great! Thank you so much.

* upstream/master: Fix statement in changelog [WIP] Add Text File Export for "Find Unlinked Files" (#3979)

* upstream/master: New translations JabRef_en.properties (French) (#4009) Fix Look and Feel related issues (#4002) Fix statement in changelog [WIP] Add Text File Export for "Find Unlinked Files" (#3979) # Conflicts: # src/main/java/org/jabref/JabRefGUI.java # src/main/java/org/jabref/gui/preftabs/AppearancePrefsTab.java # src/main/java/org/jabref/migrations/PreferencesMigrations.java

* upstream/maintable-beta: set look and feel to windows, aqua or nimbus for swing in case fix import remove look and feel New translations JabRef_en.properties (French) (#4009) Fix Look and Feel related issues (#4002) Fix statement in changelog [WIP] Add Text File Export for "Find Unlinked Files" (#3979)

…rsectionnew * upstream/maintable-beta: (88 commits) set look and feel to windows, aqua or nimbus for swing in case fix import remove look and feel New translations JabRef_en.properties (French) (#4009) Fix Look and Feel related issues (#4002) Fix statement in changelog [WIP] Add Text File Export for "Find Unlinked Files" (#3979) Fix IEEE preview does not display month (#3983) Activate context menu on key press (#4004) Improve code layout Fix concurrent modification exception when adding entries to groups Fix build Typo Add fix Rename test Fix #3994 Duplicate unmodifiable list for sorting (#3996) Remove deprecated and unused method (#3993) Improvements around external file types (#3887) Migrate to native gradle test task (#3987) Do not run checkstyle as part of the gradle check task (#3985) ...

…drop * upstream/maintable-beta: (174 commits) Fix ACM fetcher import of entries (#4019) Reimplement tooltips for file and identifier columns (#4011) Add button-icon for union/intersection in the groups side panel (#3954) Update Dependencies (#4012) set look and feel to windows, aqua or nimbus for swing in case fix import remove look and feel New translations JabRef_en.properties (French) (#4009) Fix Look and Feel related issues (#4002) Fix statement in changelog [WIP] Add Text File Export for "Find Unlinked Files" (#3979) Fix IEEE preview does not display month (#3983) Activate context menu on key press (#4004) Improve code layout Fix concurrent modification exception when adding entries to groups Fix build Typo Add fix Rename test Fix #3994 Duplicate unmodifiable list for sorting (#3996) ... # Conflicts: # src/main/java/org/jabref/gui/actions/CleanupAction.java # src/main/java/org/jabref/gui/maintable/MainTable.java

Add Text File Export for "Find Unlinked Files"

4017bbe

Siedlerchr requested changes Apr 25, 2018

View reviewed changes

Siedlerchr reviewed Apr 25, 2018

View reviewed changes

Add Text File Export for "Find Unlinked Files"

f66fa01

Add Text File Export for "Find Unlinked Files"

d5a78d0

koppor reviewed Apr 26, 2018

View reviewed changes

Merge branch 'master' into add-text-export-feature

e2670f0

tobiasdiez requested changes Apr 26, 2018

View reviewed changes

jssander added 2 commits April 27, 2018 15:44

Add Text File Export for "Find Unlinked Files"

ee09704

Add Text File Export for "Find Unlinked Files"

d78dde4

Merge branch 'master' into add-text-export-feature

194d76c

tobiasdiez approved these changes May 2, 2018

View reviewed changes

tobiasdiez merged commit ed2ff31 into JabRef:master May 2, 2018

Siedlerchr added a commit that referenced this pull request May 2, 2018

Merge remote-tracking branch 'upstream/master' into removeGTK

8d27250

* upstream/master: Fix statement in changelog [WIP] Add Text File Export for "Find Unlinked Files" (#3979)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add Text File Export for "Find Unlinked Files" #3979

[WIP] Add Text File Export for "Find Unlinked Files" #3979

jssander commented Apr 25, 2018

Siedlerchr left a comment

Siedlerchr Apr 25, 2018

Siedlerchr Apr 25, 2018

Siedlerchr Apr 25, 2018

jssander Apr 25, 2018

Siedlerchr Apr 25, 2018

jssander Apr 25, 2018

Siedlerchr Apr 25, 2018

jssander Apr 25, 2018

jssander commented Apr 25, 2018

koppor left a comment

koppor Apr 26, 2018

koppor Apr 26, 2018

koppor Apr 26, 2018

koppor Apr 26, 2018

koppor Apr 26, 2018

koppor Apr 26, 2018

koppor commented Apr 26, 2018

tobiasdiez left a comment

tobiasdiez Apr 26, 2018

tobiasdiez Apr 26, 2018

tobiasdiez Apr 26, 2018

jssander commented Apr 27, 2018

jssander commented May 1, 2018

koppor commented May 2, 2018

tobiasdiez left a comment

jssander commented May 2, 2018


		CheckableTreeNode root = (CheckableTreeNode) treeModel.getRoot();

		final List<File> fileList = getFileListFromNode(root);


		Optional<Path> exportPath = DefaultTaskExecutor
		.runInJavaFXThread(() -> ds.showFileSaveDialog(fileDialogConfiguration));

[WIP] Add Text File Export for "Find Unlinked Files" #3979

[WIP] Add Text File Export for "Find Unlinked Files" #3979

Conversation

jssander commented Apr 25, 2018

Siedlerchr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jssander commented Apr 25, 2018

koppor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koppor commented Apr 26, 2018

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jssander commented Apr 27, 2018

jssander commented May 1, 2018

koppor commented May 2, 2018

tobiasdiez left a comment

Choose a reason for hiding this comment

jssander commented May 2, 2018