-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix/copy linked files on entry transfer #13535
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
base: main
Are you sure you want to change the base?
Changes from 29 commits
7b35786
507c203
327e8b3
80a5dd8
4a94235
39e0484
2a58afb
4024b48
87a064f
f864dc3
638bc91
593d36f
f6cf22f
f53d4cc
d4871af
7385bd0
52fd290
4b3011f
2a9d039
646da11
b06fbbc
3f03974
a85b157
a0dcacd
0de8f52
9cc27f7
16bfa81
a734930
2766ba9
98b0a24
0309d35
ee18968
582c392
031f835
17d97b3
397efaf
f0e59a0
540fb72
1e4e08c
3863c2c
30c6ef5
ac3cdd3
11d7dbe
ae369d5
dd02975
cd2cbf5
6e24f48
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,21 @@ | ||
# File Transfer Between Bib Entries | ||
|
||
## File is reachable and should not be copied | ||
`req~logic.externalfiles.file-transfer.reachable-no-copy~1` | ||
When a linked file is reachable from the target context, the system must adjust the relative path in the target entry but must not copy the file again. | ||
|
||
Needs: impl | ||
|
||
## File is not reachable, but the path is the same | ||
`req~logic.externalfiles.file-transfer.not-reachable-same-path~1` | ||
When a linked file is not reachable from the target context, and the relative path in both source and target entry is the same, the file must be copied to the target context. | ||
|
||
Needs: impl | ||
|
||
## File is not reachable, and a different path is used | ||
`req~logic.externalfiles.file-transfer.not-reachable-different-path~1` | ||
When a linked file is not reachable from the target context, and the relative path differs between source and target entries, the file must be copied and the directory structure must be created to preserve the relative link. | ||
|
||
Needs: impl | ||
|
||
<!-- markdownlint-disable-file MD022 --> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
import java.awt.datatransfer.UnsupportedFlavorException; | ||
import java.io.IOException; | ||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
import javafx.application.Platform; | ||
import javafx.scene.control.TextInputControl; | ||
|
@@ -19,6 +20,7 @@ | |
import org.jabref.logic.bibtex.BibEntryWriter; | ||
import org.jabref.logic.bibtex.FieldWriter; | ||
import org.jabref.logic.preferences.CliPreferences; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
import org.jabref.model.database.BibDatabaseMode; | ||
import org.jabref.model.entry.BibEntry; | ||
import org.jabref.model.entry.BibEntryTypesManager; | ||
|
@@ -36,8 +38,11 @@ public class ClipBoardManager { | |
private static final Logger LOGGER = LoggerFactory.getLogger(ClipBoardManager.class); | ||
|
||
private static Clipboard clipboard; | ||
|
||
private static java.awt.datatransfer.Clipboard primary; | ||
|
||
private BibDatabaseContext sourceDatabaseContext; | ||
|
||
public ClipBoardManager() { | ||
this(Clipboard.getSystemClipboard(), Toolkit.getDefaultToolkit().getSystemSelection()); | ||
} | ||
|
@@ -117,6 +122,10 @@ public static String getContentsPrimary() { | |
return getContents(); | ||
} | ||
|
||
public Optional<BibDatabaseContext> getSourceBibDatabaseContext() { | ||
return Optional.ofNullable(sourceDatabaseContext); | ||
} | ||
|
||
/** | ||
* Puts content onto the system clipboard. | ||
* | ||
|
@@ -166,6 +175,12 @@ public void setContent(List<BibEntry> entries, BibEntryTypesManager entryTypesMa | |
setContent(builder.toString()); | ||
} | ||
|
||
public void setSourceBibDatabaseContext(BibDatabaseContext context) { | ||
if (context != null) { | ||
sourceDatabaseContext = context; | ||
} | ||
} | ||
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. Method allows null input despite not being necessary for operation. This violates the principle of fail-fast and contradicts modern Java practices of explicit null handling. 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. Why is there no unset of the If null is never passed: add 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. Thanks, updated as suggested. |
||
|
||
private String serializeEntries(List<BibEntry> entries, BibEntryTypesManager entryTypesManager) throws IOException { | ||
CliPreferences preferences = Injector.instantiateModelOrService(CliPreferences.class); | ||
// BibEntry is not Java serializable. Thus, we need to do the serialization manually | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,7 @@ | |
import org.jabref.gui.collab.DatabaseChangeMonitor; | ||
import org.jabref.gui.dialogs.AutosaveUiManager; | ||
import org.jabref.gui.exporter.SaveDatabaseAction; | ||
import org.jabref.gui.externalfiles.EntryImportHandlerTracker; | ||
import org.jabref.gui.externalfiles.ImportHandler; | ||
import org.jabref.gui.fieldeditors.LinkedFileViewModel; | ||
import org.jabref.gui.importer.actions.OpenDatabaseAction; | ||
|
@@ -60,6 +61,7 @@ | |
import org.jabref.logic.ai.AiService; | ||
import org.jabref.logic.citationstyle.CitationStyleCache; | ||
import org.jabref.logic.command.CommandSelectionTab; | ||
import org.jabref.logic.externalfiles.LinkedFileTransferHelper; | ||
import org.jabref.logic.importer.FetcherClientException; | ||
import org.jabref.logic.importer.FetcherException; | ||
import org.jabref.logic.importer.FetcherServerException; | ||
|
@@ -822,21 +824,52 @@ private int doCopyEntry(List<BibEntry> selectedEntries) { | |
} | ||
} | ||
|
||
public void pasteEntry() { | ||
List<BibEntry> entriesToAdd; | ||
String content = ClipBoardManager.getContents(); | ||
entriesToAdd = importHandler.handleBibTeXData(content); | ||
if (entriesToAdd.isEmpty()) { | ||
entriesToAdd = handleNonBibTeXStringData(content); | ||
} | ||
if (entriesToAdd.isEmpty()) { | ||
return; | ||
} | ||
public void pasteEntry() { | ||
String content = ClipBoardManager.getContents(); | ||
List<BibEntry> entriesToAdd = importHandler.handleBibTeXData(content); | ||
if (entriesToAdd.isEmpty()) { | ||
entriesToAdd = handleNonBibTeXStringData(content); | ||
} | ||
if (entriesToAdd.isEmpty()) { | ||
return; | ||
} | ||
copyEntriesWithFeedback(entriesToAdd); | ||
} | ||
|
||
importHandler.importEntriesWithDuplicateCheck(bibDatabaseContext, entriesToAdd); | ||
private void copyEntriesWithFeedback(List<BibEntry> entriesToAdd) { | ||
final List<BibEntry> finalEntriesToAdd = entriesToAdd; | ||
|
||
EntryImportHandlerTracker tracker = new EntryImportHandlerTracker(finalEntriesToAdd.size()); | ||
|
||
tracker.setOnFinish(() -> { | ||
int importedCount = tracker.getImportedCount(); | ||
int skippedCount = tracker.getSkippedCount(); | ||
|
||
String targetName = bibDatabaseContext.getDatabasePath() | ||
.map(path -> path.getFileName().toString()) | ||
.orElse(Localization.lang("target library")); | ||
|
||
if (importedCount == finalEntriesToAdd.size()) { | ||
dialogService.notify(Localization.lang("Pasted %0 entry(s) to %1", | ||
String.valueOf(importedCount), targetName)); | ||
} else if (importedCount == 0) { | ||
dialogService.notify(Localization.lang("No entry was pasted to %0", targetName)); | ||
} else { | ||
dialogService.notify(Localization.lang("Pasted %0 entry(s) to %1. %2 were skipped", | ||
String.valueOf(importedCount), targetName, String.valueOf(skippedCount))); | ||
} | ||
}); | ||
|
||
importHandler.importEntriesWithDuplicateCheck(bibDatabaseContext, finalEntriesToAdd, tracker); | ||
|
||
if (clipBoardManager.getSourceBibDatabaseContext().isPresent()) { | ||
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 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. Done. |
||
tracker.setOnFinish(() -> LinkedFileTransferHelper | ||
.adjustLinkedFilesForTarget(clipBoardManager.getSourceBibDatabaseContext().get(), | ||
bibDatabaseContext, preferences.getFilePreferences())); | ||
} | ||
} | ||
|
||
private List<BibEntry> handleNonBibTeXStringData(String data) { | ||
private List<BibEntry> handleNonBibTeXStringData(String data) { | ||
try { | ||
return this.importHandler.handleStringData(data); | ||
} catch (FetcherException exception) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,7 +73,7 @@ public static ContextMenu create(BibEntryTableViewModel entry, | |
|
||
contextMenu.getItems().addAll( | ||
factory.createMenuItem(StandardActions.COPY, new EditAction(StandardActions.COPY, () -> libraryTab, stateManager, undoManager)), | ||
createCopySubMenu(factory, dialogService, stateManager, preferences, clipBoardManager, abbreviationRepository, taskExecutor), | ||
createCopySubMenu(factory, dialogService, stateManager, preferences, libraryTab, clipBoardManager, abbreviationRepository, taskExecutor), | ||
createCopyToMenu(factory, dialogService, stateManager, preferences, libraryTab, importHandler), | ||
factory.createMenuItem(StandardActions.PASTE, new EditAction(StandardActions.PASTE, () -> libraryTab, stateManager, undoManager)), | ||
factory.createMenuItem(StandardActions.CUT, new EditAction(StandardActions.CUT, () -> libraryTab, stateManager, undoManager)), | ||
|
@@ -157,7 +157,8 @@ private static Menu createCopyToMenu(ActionFactory factory, | |
copyToMenu.getItems().addAll( | ||
factory.createCustomMenuItem( | ||
StandardActions.COPY_TO, | ||
new CopyTo(dialogService, stateManager, preferences.getCopyToPreferences(), importHandler, sourceDatabaseContext, bibDatabaseContext), | ||
new CopyTo(dialogService, stateManager, preferences.getCopyToPreferences(), | ||
preferences.getFilePreferences(), importHandler, sourceDatabaseContext, bibDatabaseContext), | ||
destinationDatabaseName | ||
) | ||
); | ||
|
@@ -171,11 +172,14 @@ private static Menu createCopySubMenu(ActionFactory factory, | |
DialogService dialogService, | ||
StateManager stateManager, | ||
GuiPreferences preferences, | ||
LibraryTab libraryTab, | ||
ClipBoardManager clipBoardManager, | ||
JournalAbbreviationRepository abbreviationRepository, | ||
TaskExecutor taskExecutor) { | ||
Menu copySpecialMenu = factory.createMenu(StandardActions.COPY_MORE); | ||
|
||
clipBoardManager.setSourceBibDatabaseContext(libraryTab.getBibDatabaseContext()); | ||
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 toatally wrong. Every thought of switchting tabs? Ever thought about the order of calls? Ever thoght about that there are key bindings such as Ctrl+C? --> move this to 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. Done. I hope the new approach is okay. At least the automatic and manual tests aren't breaking. |
||
|
||
copySpecialMenu.getItems().addAll( | ||
factory.createMenuItem(StandardActions.COPY_TITLE, new CopyMoreAction(StandardActions.COPY_TITLE, dialogService, stateManager, clipBoardManager, preferences, abbreviationRepository)), | ||
factory.createMenuItem(StandardActions.COPY_KEY, new CopyMoreAction(StandardActions.COPY_KEY, dialogService, stateManager, clipBoardManager, preferences, abbreviationRepository)), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,17 +113,14 @@ | |
exports org.jabref.logic.git.status; | ||
exports org.jabref.logic.command; | ||
|
||
requires java.base; | ||
|
||
requires javafx.base; | ||
requires javafx.base; | ||
requires javafx.graphics; // because of javafx.scene.paint.Color | ||
requires afterburner.fx; | ||
requires com.tobiasdiez.easybind; | ||
|
||
// for java.awt.geom.Rectangle2D required by org.jabref.logic.pdf.TextExtractor | ||
requires java.desktop; | ||
|
||
// SQL | ||
// SQL | ||
requires java.sql; | ||
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. ?! 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 dont know why this happened. the file was not touched by me. it is also not in my commit history 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. maybe during rebase or something? idk 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. Fixed indentation and removal issues in module-info.java. Not ideal for commit history, but a straightforward fix. If you prefer a different approach, please let me know. |
||
requires java.sql.rowset; | ||
|
||
|
@@ -255,6 +252,6 @@ | |
requires mslinks; | ||
requires org.antlr.antlr4.runtime; | ||
requires org.libreoffice.uno; | ||
requires org.jetbrains.annotations; | ||
// endregion | ||
requires org.jetbrains.annotations; | ||
// endregion | ||
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. Why is the indent changed? Is this some Cursor generated code? 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'm using Intellij Idea. I have made use of auto-import of the IDE. Don't know why this file was touched. 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. Fixed indentation and removal issues in module-info.java. Not ideal for commit history, but a straightforward fix. If you prefer a different approach, please let me know. |
||
} |
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.
Please keep setters and getters together.
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.
Done.