-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix threading cleanup in performSearch #7672
Changes from 2 commits
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 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,6 +3,7 @@ | |||||||||||
import java.io.File; | ||||||||||||
import java.util.List; | ||||||||||||
import java.util.Optional; | ||||||||||||
import java.util.stream.Collectors; | ||||||||||||
|
||||||||||||
import javax.swing.undo.UndoManager; | ||||||||||||
|
||||||||||||
|
@@ -23,9 +24,11 @@ | |||||||||||
import org.jabref.gui.util.TaskExecutor; | ||||||||||||
import org.jabref.logic.database.DatabaseMerger; | ||||||||||||
import org.jabref.logic.database.DuplicateCheck; | ||||||||||||
import org.jabref.logic.importer.ImportCleanup; | ||||||||||||
import org.jabref.logic.importer.ParserResult; | ||||||||||||
import org.jabref.logic.l10n.Localization; | ||||||||||||
import org.jabref.model.database.BibDatabaseContext; | ||||||||||||
import org.jabref.model.database.BibDatabaseMode; | ||||||||||||
import org.jabref.model.entry.BibEntry; | ||||||||||||
import org.jabref.model.entry.BibEntryTypesManager; | ||||||||||||
import org.jabref.model.entry.LinkedFile; | ||||||||||||
|
@@ -52,6 +55,7 @@ public class ImportEntriesViewModel extends AbstractViewModel { | |||||||||||
private final PreferencesService preferences; | ||||||||||||
private final BibEntryTypesManager entryTypesManager; | ||||||||||||
|
||||||||||||
private final ImportCleanup cleanup = new ImportCleanup(BibDatabaseMode.BIBTEX); | ||||||||||||
/** | ||||||||||||
* @param databaseContext the database to import into | ||||||||||||
* @param task the task executed for parsing the selected files(s). | ||||||||||||
|
@@ -78,10 +82,13 @@ public ImportEntriesViewModel(BackgroundTask<ParserResult> task, | |||||||||||
this.message.bind(task.messageProperty()); | ||||||||||||
|
||||||||||||
task.onSuccess(parserResult -> { | ||||||||||||
|
||||||||||||
// store the complete parser result (to import groups, ... later on) | ||||||||||||
this.parserResult = parserResult; | ||||||||||||
|
||||||||||||
List<BibEntry> resultEntries = parserResult.getDatabase().getEntries().stream().map(cleanup::doPostCleanup).collect(Collectors.toList()); | ||||||||||||
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. An idea would be to move this cleanup to the 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. Is already part of the importHandler for importing external files jabref/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java Lines 165 to 169 in bb011c9
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. Okay, then I don't get why you added it in the dialog. The 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. Ah thanks, then it can be removed here. Totally overlooked that this is called. |
||||||||||||
// fill in the list for the user, where one can select the entries to import | ||||||||||||
entries.addAll(parserResult.getDatabase().getEntries()); | ||||||||||||
entries.addAll(resultEntries); | ||||||||||||
}).onFailure(ex -> { | ||||||||||||
LOGGER.error("Error importing", ex); | ||||||||||||
dialogService.showErrorDialogAndWait(ex); | ||||||||||||
|
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.
If one uses the user database, then this would immediately take care of #1018 as well, right?