Fix for issue 10042 #12854

dennisarimi · 2025-03-31T05:20:07Z

Description

This pull request addresses the issue where, when an entry is copied from one library to another, the linked files are not copied over. The linked files need to be stored according to the destination library directory settings + file path, as opposed to source library directory settings + file path. The latter is the current implementation, which results in losing the file(s) or making them inaccessible.

Our change adds the default behaviour of copying over linked files to a different library when copying over an entry. This setting can later be disabled depending on the user's preference, through the Linked Files tab in Preferences.

Additionally, we have introduced a preference for setting a default file path if the database file path is inaccessible. This helps prevent issues when database-specific paths are unavailable or incorrectly stored.

Change Locations

src/main/.../gui/LibraryTab.java
src/main/.../gui/desktop/os/NativeDesktop.java
src/main/.../gui/preferences/linkedfiles/LinkedFilesTab.fxml
src/main/.../gui/preferences/linkedfiles/LinkedFilesTab.java
src/main/.../gui/preferences/linkedfiles/LinkedFilesTabViewModel.java
src/main/.../logic/FilePreferences.java
src/main/.../logic/preferences/JabRefCliPreferences.java
src/main/resources/l10n/JabRef_en.properties

Added a checkbox to toggle the copy setting and a text field to store the destination library directory path within which the linked files will be copied. Also added a preference for a default file path if the database file path is inaccessible.

Next Steps

While working on adding a default file path if the database file path is inaccessible, we faced an issue where database.getMetaData().getLibrarySpecificFileDirectory(); currently returns Optional.empty every time. Due to time constraints, we were unable to look more into this, and an investigation as to what might be the issue may be required.

Tests

We used manual testing to determine if the file path name was being changed when adding entries to the shared library

Screenshots

Linked Files tab in Preferences with updated features

Setting a default file path if database file path is inaccessible

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (if change is visible to the user)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

* Localize string, add preview header Signed-off-by: subhramit <[email protected]> * Add changelog entry Signed-off-by: subhramit <[email protected]> --------- Signed-off-by: subhramit <[email protected]>

koppor · 2025-03-31T17:35:08Z

Hi, my team and I have been working on this issue as a university project and have added the UI change to include a checkbox to toggle the setting in the Linked Files tab in Preferences. We however have not been able to set it to be the default state

Did you search for "default" in JabRef's code? Hint: org.jabref.gui.preferences.JabRefGuiPreferences#JabRefGuiPreferences

I replied here, because the question belongs more to the code than to the issue itself.

dennisarimi · 2025-03-31T19:43:20Z

org.jabref.gui.preferences.JabRefGuiPreferences#JabRefGuiPreferences

Hi, yes, I looked into the JabRefGuiPreferences file. However, there is no gui/linkedfiles/LinkedFilesPreferences similar to say gui/groups/GroupsPreferences or gui/entryeditors/EntryEditorPreferences, and was wondering how the linked files preferences, such as autolinking files with names starting with the citation key or moving deleted files to trash are set. (I could not find where any of the linked file preferences are set either together or separately)

I may be overlooking something simple, but in this case, would it require me to create the gui/linkedfile/LinkedFilesPreferences and refactor?

Siedlerchr · 2025-03-31T19:51:25Z

AUTOLINK_EXACT_KEY_ONLY is in CliPreferences, use double shift in Intellij so search for stuff

trag-bot · 2025-04-01T01:04:44Z

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

+            storagepath += preferences.getFilePreferences().getLinkedFileDirectory();
+        }
+
+        Pattern pattern = Pattern.compile("/");


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

trag-bot · 2025-04-01T01:04:46Z

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

+        if (generalFileDirectory.isPresent()) {
+            storagepath += generalFileDirectory.get();
+        } else {
+            storagepath += preferences.getFilePreferences().getLinkedFileDirectory();
+        }


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

dennisarimi · 2025-04-01T03:31:01Z

AUTOLINK_EXACT_KEY_ONLY is in CliPreferences, use double shift in Intellij so search for stuff

Thank you!

trag-bot · 2025-04-01T04:12:16Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java

+    public BooleanProperty copyLinkedFilesProperty() {
+        return this.copyLinkedFilesProperty;
+    }


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

trag-bot · 2025-04-01T04:12:31Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java

+    public StringProperty linkedFileDirectoryProperty() {
+        return linkedFileDirectoryProperty;
+    }


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

jabref-machine · 2025-04-01T04:13:18Z

Do not force-push! Force pushing is a very bad practice when working together on a project (mainly because it is not supported well by GitHub itself). Commits are lost and comments on commits lose their context, thus making it harder to review changes. At the end, all commits will be squashed anyway before being merged into the main branch.

trag-bot · 2025-04-01T04:13:30Z

src/main/java/org/jabref/logic/FilePreferences.java

+    public String getLinkedFileDirectory() {
+        return linkedFileDirectory.get();
+    }


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

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

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

This is correct. See other Preferences e.g.

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

src/main/java/org/jabref/logic/FilePreferences.java

trag-bot · 2025-04-01T04:48:13Z

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

-        importHandler.importEntriesWithDuplicateCheck(bibDatabaseContext, entriesToAdd);
+        boolean checkCopyPreferences = preferences.getFilePreferences().copyLinkedFiles();
+
+        if (checkCopyPreferences && (bibDatabaseContext.getLocation() == DatabaseLocation.SHARED)) {


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

trag-bot · 2025-04-01T04:48:55Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTab.java

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

+    @FXML private CheckBox copyLinkedFiles;


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

subhramit · 2025-04-01T14:22:20Z

Please give a title to the PR that summarizes your changes/what issue it fixes.
For reference, look at the titles of other PRs.

Siedlerchr · 2025-04-13T17:34:32Z

Closing this PR due to inactivity 💤 Please reopen the PR when you continue working on this

subhramit and others added 5 commits March 30, 2025 14:33

[OO] Localize string, add preview header (JabRef#12838)

3a4d580

* Localize string, add preview header Signed-off-by: subhramit <[email protected]> * Add changelog entry Signed-off-by: subhramit <[email protected]> --------- Signed-off-by: subhramit <[email protected]>

Add checkbox setting copy entry preference

4a53ed2

changed 4 files, added copyLinkedFiles properties

7482baa

continued work on text field

de21816

Merge branch 'fix-for-issue-10042'

218d840

dennisarimi mentioned this pull request Mar 31, 2025

Copying entry to another library does not copy linked file(s) #10042

Open

2 tasks

dennisarimi and others added 2 commits March 31, 2025 16:34

Updating paste entry logic

9bb6863

working, just need to tidy up

35d272d

trag-bot bot reviewed Apr 1, 2025

View reviewed changes

Refactoring to clean code after changes made

411c380

dennisarimi force-pushed the fix-for-issue-10042 branch from b702a3d to 411c380 Compare April 1, 2025 04:08

trag-bot bot reviewed Apr 1, 2025

View reviewed changes

src/main/java/org/jabref/logic/FilePreferences.java Show resolved Hide resolved

dennisarimi marked this pull request as ready for review April 1, 2025 04:47

trag-bot bot reviewed Apr 1, 2025

View reviewed changes

Siedlerchr closed this Apr 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for issue 10042 #12854

Fix for issue 10042 #12854

dennisarimi commented Mar 31, 2025 •

edited

Loading

koppor commented Mar 31, 2025

dennisarimi commented Mar 31, 2025

Siedlerchr commented Mar 31, 2025

trag-bot bot Apr 1, 2025

trag-bot bot Apr 1, 2025

dennisarimi commented Apr 1, 2025

trag-bot bot Apr 1, 2025

trag-bot bot Apr 1, 2025

jabref-machine commented Apr 1, 2025

trag-bot bot Apr 1, 2025

koppor Apr 1, 2025

Siedlerchr Apr 1, 2025 •

edited

Loading

trag-bot bot Apr 1, 2025

trag-bot bot Apr 1, 2025

subhramit commented Apr 1, 2025

Siedlerchr commented Apr 13, 2025

Fix for issue 10042 #12854

Fix for issue 10042 #12854

Conversation

dennisarimi commented Mar 31, 2025 • edited Loading

Description

Change Locations

Next Steps

Tests

Screenshots

Mandatory checks

koppor commented Mar 31, 2025

dennisarimi commented Mar 31, 2025

Siedlerchr commented Mar 31, 2025

trag-bot bot Apr 1, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 1, 2025

Choose a reason for hiding this comment

dennisarimi commented Apr 1, 2025

trag-bot bot Apr 1, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 1, 2025

Choose a reason for hiding this comment

jabref-machine commented Apr 1, 2025

trag-bot bot Apr 1, 2025

Choose a reason for hiding this comment

koppor Apr 1, 2025

Choose a reason for hiding this comment

Siedlerchr Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

trag-bot bot Apr 1, 2025

Choose a reason for hiding this comment

trag-bot bot Apr 1, 2025

Choose a reason for hiding this comment

subhramit commented Apr 1, 2025

Siedlerchr commented Apr 13, 2025

dennisarimi commented Mar 31, 2025 •

edited

Loading

Siedlerchr Apr 1, 2025 •

edited

Loading