Skip to content

Fix for issue 10042 #12854

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

Closed
wants to merge 8 commits into from
Closed

Fix for issue 10042 #12854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3a4d580
[OO] Localize string, add preview header (#12838)
subhramit Mar 27, 2025
4a53ed2
Add checkbox setting copy entry preference
dennisarimi Mar 30, 2025
7482baa
changed 4 files, added copyLinkedFiles properties
Mar 30, 2025
de21816
continued work on text field
Mar 31, 2025
218d840
Merge branch 'fix-for-issue-10042'
dennisarimi Mar 31, 2025
9bb6863
Updating paste entry logic
dennisarimi Mar 31, 2025
35d272d
working, just need to tidy up
Apr 1, 2025
411c380
Refactoring to clean code after changes made
dennisarimi Apr 1, 2025
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
53 changes: 52 additions & 1 deletion src/main/java/org/jabref/gui/LibraryTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Optional;
import java.util.Random;
import java.util.function.Supplier;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import javax.swing.undo.UndoManager;
Expand Down Expand Up @@ -93,6 +94,7 @@
import org.jabref.model.entry.event.FieldChangedEvent;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.groups.GroupTreeNode;
import org.jabref.model.search.query.SearchQuery;
import org.jabref.model.util.DirectoryMonitor;
Expand Down Expand Up @@ -937,7 +939,56 @@ public void pasteEntry() {
return;
}

importHandler.importEntriesWithDuplicateCheck(bibDatabaseContext, entriesToAdd);
boolean checkCopyPreferences = preferences.getFilePreferences().copyLinkedFiles();

if (checkCopyPreferences && (bibDatabaseContext.getLocation() == DatabaseLocation.SHARED)) {
Copy link

Choose a reason for hiding this comment

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

The condition should follow the fail-fast principle by handling invalid states early and returning, instead of nesting logic inside else branches.

List<BibEntry> linkedFileEntry = copyLinkedFiles(entriesToAdd);
importHandler.importEntriesWithDuplicateCheck(bibDatabaseContext, linkedFileEntry);
} else {
importHandler.importEntriesWithDuplicateCheck(bibDatabaseContext, entriesToAdd);
}
}

private List<BibEntry> copyLinkedFiles(List<BibEntry> entriesToAdd) {
String storagepath = "";
Optional<String> generalFileDirectory = bibDatabaseContext.getMetaData().getLibrarySpecificFileDirectory();
if (generalFileDirectory.isPresent()) {
storagepath += generalFileDirectory.get();
} else {
storagepath += preferences.getFilePreferences().getLinkedFileDirectory();
}
Comment on lines +955 to +959
Copy link

Choose a reason for hiding this comment

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

The code should follow the fail-fast principle by handling invalid states early and avoiding else branches. This can be refactored for better readability.


Pattern pattern = Pattern.compile("/");
Copy link

Choose a reason for hiding this comment

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

The pattern is compiled inside the method and used multiple times. It should be a static final field to avoid recompilation and improve performance.

for (BibEntry entry : entriesToAdd) {
int i = 0;
String desired = "";

List<LinkedFile> entryLinkedFiles = entry.getFiles();

for (LinkedFile file : entryLinkedFiles) {
String[] parts = pattern.split(file.getLink());
String filename = parts[parts.length - 1];
Path destinationFilePath = Path.of(storagepath).resolve(filename);
Path orignalFilePath = Path.of(file.getLink());

FileUtil.copyFile(orignalFilePath, destinationFilePath, true);

filename = destinationFilePath.toString();

desired += file.getDescription() + ":" + filename + ":" + file.getFileType();

if (!file.getSourceUrl().isEmpty()) {
desired += ":" + file.getSourceUrl();
}
if (i < entryLinkedFiles.size() - 1) {
desired += ";";
} else {
entry.setField(StandardField.FILE, desired);
}
i++;
}
}
return entriesToAdd;
}

private List<BibEntry> handleNonBibTeXStringData(String data) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.util.regex.Pattern;

import org.jabref.architecture.AllowedToUseAwt;
import org.jabref.cli.JabKit;
import org.jabref.gui.ClipBoardManager;
import org.jabref.gui.DialogService;
import org.jabref.gui.externalfiletype.ExternalFileType;
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTab.fxml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,13 @@
<Label styleClass="sectionHeader" text="%Attached files"/>
<CheckBox fx:id="confirmLinkedFileDelete" text="%Show confirmation dialog when deleting attached files"/>
<CheckBox fx:id="moveToTrash" text="%Move deleted files to trash (instead of deleting them)"/>


<HBox alignment="CENTER_LEFT" spacing="10.0">
<CheckBox fx:id="copyLinkedFiles" text="%Copy/move files when copying/moving entries"/>
<Label text="%Enter default database filepath" GridPane.rowIndex="1"/>
<TextField fx:id="linkedFileDirectory" HBox.hgrow="ALWAYS"/>
</HBox>


</fx:root>
8 changes: 8 additions & 0 deletions src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,14 @@ public class LinkedFilesTab extends AbstractPreferenceTabView<LinkedFilesTabView

@FXML private ComboBox<String> fileNamePattern;
@FXML private TextField fileDirectoryPattern;

@FXML private CheckBox confirmLinkedFileDelete;
@FXML private CheckBox moveToTrash;

@FXML private CheckBox copyLinkedFiles;
Copy link

Choose a reason for hiding this comment

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

The addition of the 'copyLinkedFiles' checkbox should be accompanied by a preference migration to handle any changes in default settings.

@FXML private TextField linkedFileDirectory;


private final ControlsFxVisualizer validationVisualizer = new ControlsFxVisualizer();

public LinkedFilesTab() {
Expand All @@ -65,6 +70,9 @@ public void initialize() {
moveToTrash.selectedProperty().bindBidirectional(viewModel.moveToTrashProperty());
moveToTrash.setDisable(!NativeDesktop.get().moveToTrashSupported());

copyLinkedFiles.selectedProperty().bindBidirectional(viewModel.copyLinkedFilesProperty());
linkedFileDirectory.textProperty().bindBidirectional(viewModel.linkedFileDirectoryProperty());

autolinkFileStartsBibtex.selectedProperty().bindBidirectional(viewModel.autolinkFileStartsBibtexProperty());
autolinkFileExactBibtex.selectedProperty().bindBidirectional(viewModel.autolinkFileExactBibtexProperty());
autolinkUseRegex.selectedProperty().bindBidirectional(viewModel.autolinkUseRegexProperty());
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public class LinkedFilesTabViewModel implements PreferenceTabViewModel {
private final StringProperty fileDirectoryPatternProperty = new SimpleStringProperty();
private final BooleanProperty confirmLinkedFileDeleteProperty = new SimpleBooleanProperty();
private final BooleanProperty moveToTrashProperty = new SimpleBooleanProperty();
private final BooleanProperty copyLinkedFilesProperty = new SimpleBooleanProperty();
private final StringProperty linkedFileDirectoryProperty = new SimpleStringProperty();

private final Validator mainFileDirValidator;

Expand Down Expand Up @@ -84,6 +86,8 @@ public void setValues() {
fileDirectoryPatternProperty.setValue(filePreferences.getFileDirectoryPattern());
confirmLinkedFileDeleteProperty.setValue(filePreferences.confirmDeleteLinkedFile());
moveToTrashProperty.setValue(filePreferences.moveToTrash());
copyLinkedFilesProperty.setValue(filePreferences.copyLinkedFiles());
linkedFileDirectoryProperty.setValue(filePreferences.getLinkedFileDirectory());

// Autolink preferences
switch (autoLinkPreferences.getCitationKeyDependency()) {
Expand Down Expand Up @@ -116,6 +120,8 @@ public void storeSettings() {
autoLinkPreferences.setRegularExpression(autolinkRegexKeyProperty.getValue());
filePreferences.confirmDeleteLinkedFile(confirmLinkedFileDeleteProperty.getValue());
filePreferences.moveToTrash(moveToTrashProperty.getValue());
filePreferences.copyLinkedFiles(copyLinkedFilesProperty.getValue());
filePreferences.setLinkedFileDirectory(linkedFileDirectoryProperty.getValue());
}

ValidationStatus mainFileDirValidationStatus() {
Expand Down Expand Up @@ -192,5 +198,13 @@ public BooleanProperty confirmLinkedFileDeleteProperty() {
public BooleanProperty moveToTrashProperty() {
return this.moveToTrashProperty;
}

public BooleanProperty copyLinkedFilesProperty() {
return this.copyLinkedFilesProperty;
}
Comment on lines +202 to +204
Copy link

Choose a reason for hiding this comment

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

The method 'copyLinkedFilesProperty' should return an Optional instead of potentially returning null, adhering to modern Java practices.


public StringProperty linkedFileDirectoryProperty() {
return linkedFileDirectoryProperty;
}
Comment on lines +206 to +208
Copy link

Choose a reason for hiding this comment

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

The method 'linkedFileDirectoryProperty' should return an Optional instead of potentially returning null, following modern Java practices.

}

32 changes: 31 additions & 1 deletion src/main/java/org/jabref/logic/FilePreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ public class FilePreferences {
private final BooleanProperty confirmDeleteLinkedFile = new SimpleBooleanProperty();
private final BooleanProperty moveToTrash = new SimpleBooleanProperty();
private final BooleanProperty shouldKeepDownloadUrl = new SimpleBooleanProperty();
private final BooleanProperty copyLinkedFiles = new SimpleBooleanProperty();
private final StringProperty linkedFileDirectory = new SimpleStringProperty();

public FilePreferences(String userAndHost,
String mainFileDirectory,
Expand All @@ -45,7 +47,9 @@ public FilePreferences(String userAndHost,
Path backupDirectory,
boolean confirmDeleteLinkedFile,
boolean moveToTrash,
boolean shouldKeepDownloadUrl) {
boolean shouldKeepDownloadUrl,
boolean copyLinkedFiles,
String linkedFileDirectory) {
this.userAndHost.setValue(userAndHost);
this.mainFileDirectory.setValue(mainFileDirectory);
this.storeFilesRelativeToBibFile.setValue(storeFilesRelativeToBibFile);
Expand All @@ -59,6 +63,8 @@ public FilePreferences(String userAndHost,
this.confirmDeleteLinkedFile.setValue(confirmDeleteLinkedFile);
this.moveToTrash.setValue(moveToTrash);
this.shouldKeepDownloadUrl.setValue(shouldKeepDownloadUrl);
this.copyLinkedFiles.setValue(copyLinkedFiles);
this.linkedFileDirectory.setValue(linkedFileDirectory);
}

public String getUserAndHost() {
Expand Down Expand Up @@ -216,4 +222,28 @@ public BooleanProperty shouldKeepDownloadUrlProperty() {
public void setKeepDownloadUrl(boolean shouldKeepDownloadUrl) {
this.shouldKeepDownloadUrl.set(shouldKeepDownloadUrl);
}

public boolean copyLinkedFiles() {
return copyLinkedFiles.get();
}

public BooleanProperty copyLinkedFilesProperty() {
return copyLinkedFiles;
}

public void copyLinkedFiles(boolean copyLinkedFiles) {
this.copyLinkedFiles.set(copyLinkedFiles);
}

public String getLinkedFileDirectory() {
return linkedFileDirectory.get();
}
Comment on lines +238 to +240
Copy link

Choose a reason for hiding this comment

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

The method 'getLinkedFileDirectory' should return an Optional to avoid returning null and handle absent values safely.

Copy link
Member

Choose a reason for hiding this comment

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

JabRef works with ...Property; not with the data directly.

Please refer to other methods and class variables in that class.

Copy link
Member

@Siedlerchr Siedlerchr Apr 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is correct. See other Preferences e.g.


    public boolean shouldKeepDownloadUrl() {
        return shouldKeepDownloadUrl.get();
    }


public StringProperty linkedFileDirectoryProperty() {
return linkedFileDirectory;
}

public void setLinkedFileDirectory(String linkedFileDirectory) {
this.linkedFileDirectory.set(linkedFileDirectory);
}
}
12 changes: 11 additions & 1 deletion src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ public class JabRefCliPreferences implements CliPreferences {
public static final String UNWANTED_CITATION_KEY_CHARACTERS = "defaultUnwantedBibtexKeyCharacters";
public static final String CONFIRM_LINKED_FILE_DELETE = "confirmLinkedFileDelete";
public static final String TRASH_INSTEAD_OF_DELETE = "trashInsteadOfDelete";
public static final String COPY_LINKED_FILES = "copyLinkedFiles";
public static final String LINKED_FILEDIRPATTERN = "linkedFileDirPattern";
public static final String WARN_BEFORE_OVERWRITING_KEY = "warnBeforeOverwritingKey";
public static final String AVOID_OVERWRITING_KEY = "avoidOverwritingKey";
public static final String AUTOLINK_EXACT_KEY_ONLY = "autolinkExactKeyOnly";
Expand Down Expand Up @@ -606,6 +608,10 @@ protected JabRefCliPreferences() {
defaults.put(STORE_RELATIVE_TO_BIB, Boolean.TRUE);

defaults.put(AUTOLINK_EXACT_KEY_ONLY, Boolean.FALSE);

defaults.put(COPY_LINKED_FILES, Boolean.TRUE);
defaults.put(LINKED_FILEDIRPATTERN, "");

defaults.put(AUTOLINK_FILES_ENABLED, Boolean.TRUE);
defaults.put(LOCAL_AUTO_SAVE, Boolean.FALSE);
defaults.put(ALLOW_INTEGER_EDITION_BIBTEX, Boolean.FALSE);
Expand Down Expand Up @@ -1563,7 +1569,9 @@ public FilePreferences getFilePreferences() {
getBoolean(CONFIRM_LINKED_FILE_DELETE),
// We make use of the fallback, because we need AWT being initialized, which is not the case at the constructor JabRefPreferences()
getBoolean(TRASH_INSTEAD_OF_DELETE, moveToTrashSupported()),
getBoolean(KEEP_DOWNLOAD_URL));
getBoolean(KEEP_DOWNLOAD_URL),
getBoolean(COPY_LINKED_FILES),
get(LINKED_FILEDIRPATTERN));

EasyBind.listen(getInternalPreferences().getUserAndHostProperty(), (obs, oldValue, newValue) -> filePreferences.getUserAndHostProperty().setValue(newValue));
EasyBind.listen(filePreferences.mainFileDirectoryProperty(), (obs, oldValue, newValue) -> put(MAIN_FILE_DIRECTORY, newValue));
Expand All @@ -1578,6 +1586,8 @@ public FilePreferences getFilePreferences() {
EasyBind.listen(filePreferences.confirmDeleteLinkedFileProperty(), (obs, oldValue, newValue) -> putBoolean(CONFIRM_LINKED_FILE_DELETE, newValue));
EasyBind.listen(filePreferences.moveToTrashProperty(), (obs, oldValue, newValue) -> putBoolean(TRASH_INSTEAD_OF_DELETE, newValue));
EasyBind.listen(filePreferences.shouldKeepDownloadUrlProperty(), (obs, oldValue, newValue) -> putBoolean(KEEP_DOWNLOAD_URL, newValue));
EasyBind.listen(filePreferences.copyLinkedFilesProperty(), (obs, oldValue, newValue) -> putBoolean(COPY_LINKED_FILES, newValue));
EasyBind.listen(filePreferences.linkedFileDirectoryProperty(), (obs, oldValue, newValue) -> put(LINKED_FILEDIRPATTERN, newValue));

return filePreferences;
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2889,3 +2889,6 @@ Open\ welcome\ tab=Open welcome tab
Current\ JabRef\ version\:\ %0=Current JabRef version: %0
Download\ development\ version=Download development version
CHANGELOG=CHANGELOG

Copy/move\ files\ when\ copying/moving\ entries=Copy/move files when copying/moving entries
Enter\ default\ database\ filepath=Enter default database filepath