Skip to content
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.

Sign up for GitHub

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

[WIP] Batch Insert entries #5691

Merged
merged 86 commits into from
Dec 30, 2019
Merged

[WIP] Batch Insert entries #5691

Show file tree
Hide file tree
Changes from 62 commits
Commits
Show all changes
86 commits
Select commit Hold shift + click to select a range
80e8377
Initial changes to remove entry functionality
abepolk Oct 8, 2019
48cb7fd
Fix logic error in KeyChangeListener
abepolk Oct 8, 2019
923b40e
Fix logic error in BasePanel
abepolk Oct 8, 2019
a0ffcb5
Changes to tests
abepolk Oct 9, 2019
b08ae88
Fix typo
abepolk Oct 9, 2019
3a830a8
Move remove entry calls to batch version
abepolk Oct 11, 2019
4e57a46
Un-propagate for loop in KeyChangeListener
abepolk Oct 15, 2019
b990e0d
Finalize changes to SharedEntriesNotPresentEvent
abepolk Oct 15, 2019
3122e67
Fix bug
abepolk Oct 15, 2019
aac840c
Fix other compile errors
abepolk Oct 15, 2019
90c27f1
Fix bug and update tests
abepolk Oct 20, 2019
a0cafc4
Fix tests
abepolk Oct 20, 2019
868deed
Merge master
abepolk Oct 20, 2019
2afb868
Fix test omission
abepolk Oct 20, 2019
460f44a
Update l10n
abepolk Oct 20, 2019
f11389e
For loop to citationStyle
abepolk Oct 20, 2019
f535a0b
Add comment for method not working
abepolk Oct 23, 2019
77ff450
Clarify var name
abepolk Oct 23, 2019
b0805cc
Allow single entry for undo
abepolk Oct 23, 2019
7ac7a1c
Replace compound edit undo with normal undo in BasePanel
abepolk Oct 24, 2019
8255ad2
Typo
abepolk Oct 25, 2019
3b8dfea
Simplify loop in DBMSSynchronizer
abepolk Oct 27, 2019
5f1b811
Use if instead of stream
abepolk Oct 28, 2019
c41dc17
Pluralize Javadoc
abepolk Oct 30, 2019
bb88484
EntryEvent -> EntriesEvent in Javadoc, comments, and var names
abepolk Oct 30, 2019
4c97c64
Make imports explicit in BasePanel
abepolk Oct 30, 2019
30d8d51
Merge master
abepolk Oct 30, 2019
125d33a
Batch delete to SQL
abepolk Nov 1, 2019
5cf5965
Final EntriesRemovedEvent fixes
abepolk Nov 1, 2019
1df2869
Fix checkStyleMain
abepolk Nov 3, 2019
0f5d76f
Merge branch 'master' into refactor_group_entryevents
abepolk Nov 3, 2019
13abf0c
More checkStyle fixes
abepolk Nov 3, 2019
455ce0c
Initial changes to batch insertEntries calls
abepolk Nov 10, 2019
17f04b0
More fixes
abepolk Nov 10, 2019
30e5087
Merge remove_entry fixes
abepolk Nov 10, 2019
8ae5098
More batching insertEntry
abepolk Nov 10, 2019
214e958
Move List coercion into DuplicateSearchResult method
abepolk Nov 10, 2019
5f0e7f5
Even more batching insertEntry
abepolk Nov 10, 2019
2824aed
DefaultAuxParser batch insertEntry
abepolk Nov 10, 2019
940bf78
MrDLibImporter batch insertEntry
abepolk Nov 10, 2019
3e71219
Add comment about usage of BasePanel.insertEntry
abepolk Nov 10, 2019
a90c44e
Fix typo
abepolk Nov 11, 2019
648c5da
Fix pesky BibDatabaseTest error with setStrings
abepolk Nov 11, 2019
b0f585a
Fixed BibDatabase Javadoc
abepolk Nov 11, 2019
c6300e5
Merge master
abepolk Nov 18, 2019
a187b42
Finish master merge, start changing DBMS tests
abepolk Nov 20, 2019
d6d15a7
Merge refactor_group_entryevents
abepolk Nov 22, 2019
f8c94cc
Add comment
abepolk Nov 22, 2019
aaf73e7
Final fixes including checkStyle
abepolk Nov 24, 2019
120b6d5
Add DBMSProcessor tests
abepolk Nov 26, 2019
b023741
Fix checkStyleTest
abepolk Nov 26, 2019
ef18bb0
Merge branch 'insert_entries' into batch_remove_entries
abepolk Nov 29, 2019
f74717f
Merge branch 'master' into insert_entries
abepolk Nov 29, 2019
4adc9b0
Finished batching calls to insertEntries
abepolk Dec 2, 2019
3890e9d
Minor change for consistency
abepolk Dec 2, 2019
2aa889a
Update comments
abepolk Dec 2, 2019
dfe273e
Update comment
abepolk Dec 2, 2019
fe3f830
Another update comment
abepolk Dec 2, 2019
1fd76fd
EntryAddedEvent -> EntriesAddedEvnet and removed AllInsertsFinishedEvent
abepolk Dec 2, 2019
0e9f6a2
Fixed some tests, aux parser still failing
abepolk Dec 2, 2019
b7bb130
AuxParserTest passes now
abepolk Dec 2, 2019
153fab3
Fix checkstyle
abepolk Dec 3, 2019
6400f98
Merge branch 'master' into insert_entries
abepolk Dec 3, 2019
ee1bbc2
Make corrections
abepolk Dec 13, 2019
250d405
Typo
abepolk Dec 13, 2019
8913d4b
More streaming
abepolk Dec 13, 2019
d673f55
Add UndoableInsertEntries constructor
abepolk Dec 13, 2019
ce86e71
Fix bug
abepolk Dec 13, 2019
5720593
Get rid of "Integrity Check Failed" info
abepolk Dec 15, 2019
0601c4c
Merge branch 'master' into insert_entries
abepolk Dec 15, 2019
d8e3ac2
Properly close databases in test to avoid database listener errors
abepolk Dec 15, 2019
772f8f6
Fix database tests by adding entries after firing EntryAddedEvent
abepolk Dec 23, 2019
454ccd0
Remove redundant DBMS connection closure from DBMSSychronizerTest
abepolk Dec 23, 2019
2fb860a
Merge test resources from master
abepolk Dec 23, 2019
039e223
Merge master
abepolk Dec 23, 2019
790e7f2
Remove firstEntry hack
abepolk Dec 24, 2019
76845c2
Remove duplicate code from paste() by and pluralize insertEntries() i…
abepolk Dec 24, 2019
30fc8e5
Comment fix
abepolk Dec 24, 2019
a192c2f
Clarify BasePanel insertEntries JavaDoc
abepolk Dec 24, 2019
a74381f
Rename vars
abepolk Dec 24, 2019
f2a517f
Rename vars
abepolk Dec 24, 2019
3f78aa9
Remove unnecessary TODO and rename EntriesRemovedListener
abepolk Dec 24, 2019
d42890e
Merge master
abepolk Dec 30, 2019
85ff37b
Merge fixes
abepolk Dec 30, 2019
659db97
Remove old comment
abepolk Dec 30, 2019
a2a619c
checkStyle fixes
abepolk Dec 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions src/main/java/org/jabref/gui/BasePanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
import org.jabref.gui.undo.CountingUndoManager;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableFieldChange;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.gui.undo.UndoableRemoveEntries;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.worker.SendAsEMailAction;
Expand All @@ -80,8 +80,8 @@
import org.jabref.model.database.KeyCollisionException;
import org.jabref.model.database.event.BibDatabaseContextChangedEvent;
import org.jabref.model.database.event.CoarseChangeFilter;
import org.jabref.model.database.event.EntriesAddedEvent;
import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.database.event.EntryAddedEvent;
import org.jabref.model.database.shared.DatabaseLocation;
import org.jabref.model.database.shared.DatabaseSynchronizer;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -610,6 +610,8 @@ public void registerUndoableChanges(List<FieldChange> changes) {
*
* @param bibEntry The new entry.
*/

// The Javadoc appears to be incorrect here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then please fix it or just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the question below. I am trying to ascertain the purpose of the method so I understand it.

public void insertEntry(final BibEntry bibEntry) {
if (bibEntry != null) {
try {
Expand All @@ -618,8 +620,8 @@ public void insertEntry(final BibEntry bibEntry) {
// Set owner and timestamp
UpdateField.setAutomaticFields(bibEntry, true, true, Globals.prefs.getUpdateFieldPreferences());

// Create an UndoableInsertEntry object.
getUndoManager().addEdit(new UndoableInsertEntry(bibDatabaseContext.getDatabase(), bibEntry));
// Create an UndoableInsertEntries object.
getUndoManager().addEdit(new UndoableInsertEntries(bibDatabaseContext.getDatabase(), Collections.singletonList(bibEntry)));

markBaseChanged(); // The database just changed.
if (Globals.prefs.getBoolean(JabRefPreferences.AUTO_OPEN_FORM)) {
Expand Down Expand Up @@ -1119,17 +1121,16 @@ public void searchAndOpen() {
private class GroupTreeListener {

@Subscribe
public void listen(EntryAddedEvent addedEntryEvent) {
// if the added entry is an undo don't add it to the current group
if (addedEntryEvent.getEntriesEventSource() == EntriesEventSource.UNDO) {
public void listen(EntriesAddedEvent addedEntriesEvent) {
// if the event is an undo, don't add it to the current group
if (addedEntriesEvent.getEntriesEventSource() == EntriesEventSource.UNDO) {
return;
}

// Automatically add new entry to the selected group (or set of groups)
// Automatically add new entries to the selected group (or set of groups)
if (Globals.prefs.getBoolean(JabRefPreferences.AUTO_ASSIGN_GROUP)) {
final List<BibEntry> entries = Collections.singletonList(addedEntryEvent.getBibEntry());
Globals.stateManager.getSelectedGroup(bibDatabaseContext).forEach(
selectedGroup -> selectedGroup.addEntriesToGroup(entries));
selectedGroup -> selectedGroup.addEntriesToGroup(addedEntriesEvent.getBibEntries()));
}
}
}
Expand All @@ -1149,8 +1150,8 @@ public void listen(EntriesRemovedEvent entriesRemovedEvent) {
private class SearchAutoCompleteListener {

@Subscribe
public void listen(EntryAddedEvent addedEntryEvent) {
DefaultTaskExecutor.runInJavaFXThread(() -> searchAutoCompleter.indexEntry(addedEntryEvent.getBibEntry()));
public void listen(EntriesAddedEvent addedEntriesEvent) {
DefaultTaskExecutor.runInJavaFXThread(() -> addedEntriesEvent.getBibEntries().forEach(entry -> searchAutoCompleter.indexEntry(entry)));
}

@Subscribe
Expand All @@ -1166,7 +1167,7 @@ public void listen(EntryChangedEvent entryChangedEvent) {
private class SearchListener {

@Subscribe
public void listen(EntryAddedEvent addedEntryEvent) {
public void listen(EntriesAddedEvent addedEntryEvent) {
DefaultTaskExecutor.runInJavaFXThread(() -> frame.getGlobalSearchBar().performSearch());
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/EntryTypeViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public void runFetcherWorker() {
} else {
// Regenerate CiteKey of imported BibEntry
new BibtexKeyGenerator(basePanel.getBibDatabaseContext(), prefs.getBibtexKeyPatternPreferences()).generateAndSetKey(entry);
basePanel.insertEntry(entry);
basePanel.insertEntry(entry); // Should this be basePanel.getDatabase().insertEntry(entry);?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The called method does more: set automatic fields, do an unduable event, marks the database changed, selects the entry.

Maybe, one does not want to have the entry marked? However, I would guess so as after a fetching (context of this method), I want to know which entry was fetched.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let me make sure I understand. It sounds like the BasePanel.insertEntry method is called when the user's action's primary purpose is to insert an entry, for example when the "insert entry" button is pressed. However, the BibDatabase.insertEntry method is called when the entry is inserted in the process of doing something else, for example when importing PDF content, and you wouldn't want an undo just for the entry insertion alone. Is that right?

Copy link
Member

@koppor koppor Dec 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobiasdiez Is this the idea? I really like the long code where the entry is updated (timestamp, owner field) and selected. This makes JabRef a better management software, not just BibTeX editing software.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think basePanel.insertEntry is indeed the better choice, because of the extra work it does. BibDatabase.insertEntry is just the plain insertion of the entry in the database.

}
searchSuccesfulProperty.set(true);

Expand Down
12 changes: 9 additions & 3 deletions src/main/java/org/jabref/gui/autocompleter/AutoCompleteUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package org.jabref.gui.autocompleter;

import org.jabref.model.database.event.EntryAddedEvent;
import java.util.List;

import org.jabref.model.database.event.EntriesAddedEvent;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.event.EntryChangedEvent;

import com.google.common.eventbus.Subscribe;
Expand All @@ -17,8 +20,11 @@ public AutoCompleteUpdater(SuggestionProviders suggestionProviders) {
}

@Subscribe
public void listen(EntryAddedEvent addedEntryEvent) {
suggestionProviders.indexEntry(addedEntryEvent.getBibEntry());
public void listen(EntriesAddedEvent addedEntryEvent) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the variable be renamed to entriesAddedEvent?

List<BibEntry> entries = addedEntryEvent.getBibEntries();
for (BibEntry entry : entries) {
suggestionProviders.indexEntry(entry);
}
}

@Subscribe
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/jabref/gui/collab/EntryAddChangeViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package org.jabref.gui.collab;

import java.util.Collections;

import javafx.scene.Node;

import org.jabref.Globals;
import org.jabref.JabRefGUI;
import org.jabref.gui.preview.PreviewViewer;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
Expand All @@ -23,7 +25,7 @@ public EntryAddChangeViewModel(BibEntry diskEntry) {
@Override
public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
database.getDatabase().insertEntry(diskEntry);
undoEdit.addEdit(new UndoableInsertEntry(database.getDatabase(), diskEntry));
undoEdit.addEdit(new UndoableInsertEntries(database.getDatabase(), Collections.singletonList(diskEntry)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having users of the method forced to wrap the parameter into Collections.singletonList, I am thinking, why isn't there an overloaded method with , ... BibEntry entry added, which alles Collections.singletonList internally? -- Reasoning: There are more callers than callees, thus make it easy for the callers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. My original purpose was to discourage users from using the singular version, but it would make it more consistent in that I have a singular insertEntry method in BibDatabase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this just require a new constructor for UndoableInsertEntries or require a whole new class UndoableInsertEntry?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new constructor should be sufficient.

}

@Override
Expand Down
8 changes: 3 additions & 5 deletions src/main/java/org/jabref/gui/duplicationFinder/DuplicateSearch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import org.jabref.gui.duplicationFinder.DuplicateResolverDialog.DuplicateResolverResult;
import org.jabref.gui.duplicationFinder.DuplicateResolverDialog.DuplicateResolverType;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.gui.undo.UndoableRemoveEntries;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.DefaultTaskExecutor;
Expand Down Expand Up @@ -167,10 +167,8 @@ private void handleDuplicates(DuplicateSearchResult result) {
}
// and adding merged entries:
if (!result.getToAdd().isEmpty()) {
for (BibEntry entry : result.getToAdd()) {
panel.getDatabase().insertEntry(entry);
compoundEdit.addEdit(new UndoableInsertEntry(panel.getDatabase(), entry));
}
compoundEdit.addEdit(new UndoableInsertEntries(panel.getDatabase(), result.getToAdd()));
panel.getDatabase().insertEntries(result.getToAdd());
panel.markBaseChanged();
}

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import org.jabref.gui.DialogService;
import org.jabref.gui.StateManager;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.logic.bibtexkeypattern.BibtexKeyGenerator;
import org.jabref.logic.externalfiles.ExternalFilesContentImporter;
import org.jabref.logic.importer.ImportFormatPreferences;
Expand Down Expand Up @@ -94,7 +94,7 @@ public void importAsNewEntries(List<Path> files) {
}

importEntries(entriesToAdd);
entriesToAdd.forEach(entry -> ce.addEdit(new UndoableInsertEntry(database.getDatabase(), entry)));
ce.addEdit(new UndoableInsertEntries(database.getDatabase(), entriesToAdd));
}
ce.end();
undoManager.addEdit(ce);
Expand All @@ -109,7 +109,7 @@ private BibEntry createEmptyEntryWithLink(Path file) {

public void importEntries(List<BibEntry> entries) {
//TODO: Add undo/redo
//ce.addEdit(new UndoableInsertEntry(panel.getDatabase(), entry));
//undoManager.addEdit(new UndoableInsertEntries(panel.getDatabase(), entries));

database.getDatabase().insertEntries(entries);

Expand Down
11 changes: 3 additions & 8 deletions src/main/java/org/jabref/gui/importer/ImportAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -135,9 +134,7 @@ private ParserResult mergeImportResults(List<ImportFormatReader.UnknownFormatImp
directParserResult = pr;
}
// Merge entries:
for (BibEntry entry : pr.getDatabase().getEntries()) {
database.insertEntry(entry);
}
database.insertEntries(pr.getDatabase().getEntries());

// Merge strings:
for (BibtexString bs : pr.getDatabase().getStringValues()) {
Expand All @@ -151,16 +148,14 @@ private ParserResult mergeImportResults(List<ImportFormatReader.UnknownFormatImp
} else {

ParserResult pr = importResult.parserResult;
Collection<BibEntry> entries = pr.getDatabase().getEntries();
List<BibEntry> entries = pr.getDatabase().getEntries();

anythingUseful = anythingUseful | !entries.isEmpty();

// set timestamp and owner
UpdateField.setAutomaticFields(entries, Globals.prefs.getUpdateFieldPreferences()); // set timestamp and owner

for (BibEntry entry : entries) {
database.insertEntry(entry);
}
database.insertEntries(entries);
}
}

Expand Down
14 changes: 6 additions & 8 deletions src/main/java/org/jabref/gui/importer/actions/AppendDatabaseAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.jabref.gui.actions.BaseAction;
import org.jabref.gui.importer.AppendDatabaseDialog;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.gui.undo.UndoableInsertString;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.FileDialogConfiguration;
Expand Down Expand Up @@ -59,8 +59,7 @@ private static void mergeFromBibtex(BasePanel panel, ParserResult parserResult,
boolean importStrings, boolean importGroups, boolean importSelectorWords) throws KeyCollisionException {

BibDatabase fromDatabase = parserResult.getDatabase();
List<BibEntry> appendedEntries = new ArrayList<>();
List<BibEntry> originalEntries = new ArrayList<>();
List<BibEntry> entriesToAppend = new ArrayList<>();
BibDatabase database = panel.getDatabase();

NamedCompound ce = new NamedCompound(Localization.lang("Append library"));
Expand All @@ -74,11 +73,10 @@ private static void mergeFromBibtex(BasePanel panel, ParserResult parserResult,
BibEntry entry = (BibEntry) originalEntry.clone();
UpdateField.setAutomaticFields(entry, overwriteOwner, overwriteTimeStamp,
Globals.prefs.getUpdateFieldPreferences());
database.insertEntry(entry);
appendedEntries.add(entry);
originalEntries.add(originalEntry);
ce.addEdit(new UndoableInsertEntry(database, entry));
entriesToAppend.add(entry);
}
database.insertEntries(entriesToAppend);
tobiasdiez marked this conversation as resolved.
Show resolved Hide resolved
ce.addEdit(new UndoableInsertEntries(database, entriesToAppend));
}

if (importStrings) {
Expand All @@ -99,7 +97,7 @@ private static void mergeFromBibtex(BasePanel panel, ParserResult parserResult,
ExplicitGroup group = new ExplicitGroup("Imported", GroupHierarchyType.INDEPENDENT,
Globals.prefs.getKeywordDelimiter());
newGroups.setGroup(group);
group.add(appendedEntries);
group.add(entriesToAppend);
} catch (IllegalArgumentException e) {
LOGGER.error("Problem appending entries to group", e);
}
Expand Down
20 changes: 7 additions & 13 deletions src/main/java/org/jabref/gui/maintable/MainTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.keyboard.KeyBinding;
import org.jabref.gui.keyboard.KeyBindingRepository;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.gui.util.ControlHelper;
import org.jabref.gui.util.CustomLocalDragboard;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.ViewModelTableRowFactory;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.UpdateField;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.database.event.AllInsertsFinishedEvent;
import org.jabref.model.database.event.EntriesAddedEvent;
import org.jabref.model.entry.BibEntry;
import org.jabref.preferences.JabRefPreferences;

Expand Down Expand Up @@ -130,8 +129,8 @@ public MainTable(MainTableDataModel model, JabRefFrame frame,
}

@Subscribe
public void listen(AllInsertsFinishedEvent event) {
DefaultTaskExecutor.runInJavaFXThread(() -> clearAndSelect(event.getBibEntry()));
public void listen(EntriesAddedEvent event) {
DefaultTaskExecutor.runInJavaFXThread(() -> clearAndSelect(event.getFirstEntry()));
}

public void clearAndSelect(BibEntry bibEntry) {
Expand Down Expand Up @@ -217,17 +216,12 @@ public void paste() {
List<BibEntry> entriesToAdd = Globals.clipboardManager.extractData();

if (!entriesToAdd.isEmpty()) {
// Add new entries
NamedCompound ce = new NamedCompound((entriesToAdd.size() > 1 ? Localization.lang("paste entries") : Localization.lang("paste entry")));
database.getDatabase().insertEntries(entriesToAdd);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the other code, I wonder why insertEntries of the basePanel is not called. We want to focus the entries, don't we?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also check if the UpdateField.setAutomaticFields call is then still required or if it's part of basePanel.insertEntries.

// TODO because of this line, when implementing batch undo insert entries, model constructor after batch remove entries with a boolean for paste
undoManager.addEdit(new UndoableInsertEntries(database.getDatabase(), entriesToAdd, true));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid Boolean parameters. See https://java.by-comparison.com/.

Please introduce an enum AdditionMode with values PASTE, INSERT. With that way, it is crystal clear what paste = false means. In other context paste = false could mean cut (or copy), so with an enum is really clear what semantics is intended.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the TODO here? 😇

for (BibEntry entryToAdd : entriesToAdd) {
UpdateField.setAutomaticFields(entryToAdd, Globals.prefs.getUpdateFieldPreferences());

database.getDatabase().insertEntry(entryToAdd);

ce.addEdit(new UndoableInsertEntry(database.getDatabase(), entryToAdd));
}
ce.end();
undoManager.addEdit(ce);

// Show editor if user want us to do this
BibEntry firstNewEntry = entriesToAdd.get(0);
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/jabref/gui/mergeentries/MergeEntriesAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jabref.gui.mergeentries;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

Expand All @@ -10,7 +11,7 @@
import org.jabref.gui.StateManager;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.gui.undo.UndoableInsertEntry;
import org.jabref.gui.undo.UndoableInsertEntries;
import org.jabref.gui.undo.UndoableRemoveEntries;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -52,12 +53,13 @@ public void execute() {
dlg.setTitle(Localization.lang("Merge entries"));
Optional<BibEntry> mergedEntry = dlg.showAndWait();
if (mergedEntry.isPresent()) {
// Should this be basePanel.getDatabase().insertEntry(mergedEntry.get());?
basePanel.insertEntry(mergedEntry.get());

// Create a new entry and add it to the undo stack
// Remove the other two entries and add them to the undo stack (which is not working...)
NamedCompound ce = new NamedCompound(Localization.lang("Merge entries"));
ce.addEdit(new UndoableInsertEntry(basePanel.getDatabase(), mergedEntry.get()));
ce.addEdit(new UndoableInsertEntries(basePanel.getDatabase(), Collections.singletonList(mergedEntry.get())));
List<BibEntry> entriesToRemove = Arrays.asList(one, two);
ce.addEdit(new UndoableRemoveEntries(basePanel.getDatabase(), entriesToRemove));
basePanel.getDatabase().removeEntries(entriesToRemove);
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/org/jabref/gui/openoffice/OOBibBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ private List<XTextDocument> getTextDocuments() throws NoSuchElementException, Wr
if (document != null) {
result.add(document);
}
}
}
return result;
}

Expand Down Expand Up @@ -1273,6 +1273,7 @@ public BibDatabase generateDatabase(List<BibDatabase> databases)
throws NoSuchElementException, WrappedTargetException {
BibDatabase resultDatabase = new BibDatabase();
List<String> cited = findCitedKeys();
List<BibEntry> entriesToInsert = new ArrayList<BibEntry>();

// For each cited key
for (String key : cited) {
Expand All @@ -1283,13 +1284,13 @@ public BibDatabase generateDatabase(List<BibDatabase> databases)
if (entry.isPresent()) {
BibEntry clonedEntry = (BibEntry) entry.get().clone();
// Insert a copy of the entry
resultDatabase.insertEntry(clonedEntry);
entriesToInsert.add(clonedEntry);
// Check if the cloned entry has a crossref field
clonedEntry.getField(StandardField.CROSSREF).ifPresent(crossref -> {
// If the crossref entry is not already in the database
if (!resultDatabase.getEntryByKey(crossref).isPresent()) {
// Add it if it is in the current library
loopDatabase.getEntryByKey(crossref).ifPresent(resultDatabase::insertEntry);
loopDatabase.getEntryByKey(crossref).ifPresent(entriesToInsert::add);
}
});

Expand All @@ -1298,7 +1299,7 @@ public BibDatabase generateDatabase(List<BibDatabase> databases)
}
}
}

resultDatabase.insertEntries(entriesToInsert);
return resultDatabase;
}

Expand Down
Loading