Skip to content

Enhancement: Move linked file to folder offers all JabRef Directories as options - #15055

Merged
Siedlerchr merged 52 commits into
JabRef:mainfrom
ganesh-vk:move-linked-files-directory
May 17, 2026
Merged

Enhancement: Move linked file to folder offers all JabRef Directories as options#15055
Siedlerchr merged 52 commits into
JabRef:mainfrom
ganesh-vk:move-linked-files-directory

Conversation

@ganesh-vk

@ganesh-vk ganesh-vk commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

Closes #12287

Steps to test

  • Open a library and configure file directories in library properties.
  • Attach a file to an entry.
  • Use Move file(s) to the next configured directory to move the file to the next configured directory.

Opened as a draft since the following needs discussion:

  • Currently the UI says Move file(s) to the next configured directory. I think this is still an improvement compared to the current status quo but the issue wanted it to mention the directory its going to be moved to. I couldn't figure out how to implement this cleanly so some hints would be appreciated or the current implementation can be accepted.
  • Others reasons mentioned below.

AI usage disclosure: Used Deepwiki to better understand the issue and discussed with ChatGPT/Gemini when necessary. All usage was in accordance with the AI usage policy.

image

Checklist

…ady in a Jabref Directory

File is moved to the next available file directory that is configured. Available directorie's are searched in the order User -> Library -> BIB
git commit -m

@ganesh-vk ganesh-vk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let me know what I can do here

Comment thread jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java Outdated
assertTrue(action.isExecutable(), "DOWNLOAD_FILE should be executable for online link");
}

@Test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This test uses the earlier implementation (which found the first configured directory in User > Library > Bib). Test fails with the new implementation so I deleted it. The UI now does not get greyed out since the check is no longer present. However the error dialog box provides specific reasons for failure instead of something generic.

@github-actions github-actions Bot added status: changes-required Pull requests that are not yet complete and removed status: changes-required Pull requests that are not yet complete labels Feb 8, 2026
@ganesh-vk

Copy link
Copy Markdown
Contributor Author

I've looked into this a little more. The text for each action is set once at build time and is not changed dynamically. Since a user can select multiple files at once and then attempt to move them all at once and since it's possible that the files can be in different directories, I think the generic message is the solution that we should use.

If two files are linked to the same entry (one present in say the User specific directory and the other in the Library specific directory) and a user selects both and opens the menu, we cannot mention a singular directory to which both files shall be moved to since they are different.

@ganesh-vk
ganesh-vk marked this pull request as ready for review February 8, 2026 19:51
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Implement file rotation across configured directories

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Implement file rotation across configured directories
  - Files now move to next available directory (User → Library → BIB)
  - Skips null/unconfigured directories in rotation cycle
  - Preserves directory structure when moving between configured directories
• Update UI text to clarify "next configured directory" behavior
• Add comprehensive test coverage for directory rotation logic
• Refactor LinkedFileHandler to support arbitrary target directories
Diagram
flowchart LR
  A["File in User Dir"] -->|moveToNextPossibleDirectory| B["Determine Current Dir Index"]
  B -->|Find Next Valid Dir| C["Skip Null Directories"]
  C -->|Rotate Cycle| D["Move to Next Dir"]
  D -->|Preserve Structure| E["File in Library/BIB Dir"]
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java ✨ Enhancement +1/-1

Update action label for clarity

• Updated MOVE_FILE_TO_FOLDER action label from "Move file(s) to file directory" to "Move file(s)
 to the next configured file directory"
• Clarifies that the action rotates through configured directories rather than moving to a single
 default

jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java


2. jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java ✨ Enhancement +73/-19

Implement directory rotation logic

• Renamed moveToDefaultDirectory() to moveToNextPossibleDirectory() to reflect new rotation
 behavior
• Implemented getNextTargetPath() method to find next valid directory in rotation cycle
• Added logic to skip null directories and handle wraparound (BIB → User)
• Preserves nested directory structure when moving between configured directories
• Enhanced error handling with specific messages for missing directories and files
• Updated moveToDefaultDirectoryAndRename() to call renamed method

jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java


3. jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextAction.java ✨ Enhancement +7/-17

Consolidate action binding logic

• Consolidated binding logic for MOVE_FILE_TO_FOLDER, OPEN_FILE, OPEN_FOLDER,
 RENAME_FILE_TO_NAME, and DELETE_FILE actions
• Removed condition !linkedFile.isGeneratedPathSameAsOriginal() from move action enablement
• Updated method call from moveToDefaultDirectory() to moveToNextPossibleDirectory()
• Simplified action executability conditions

jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextAction.java


View more (5)
4. jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelFileRotationTest.java 🧪 Tests +200/-0

Add directory rotation test suite

• Added comprehensive test suite for directory rotation functionality
• Tests rotation through all three directory types (User → Library → BIB → User)
• Tests skipping of null/unconfigured directories
• Tests directory structure preservation for nested files
• Tests error handling when no alternative directory is configured
• Tests behavior when file is outside all configured directories

jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelFileRotationTest.java


5. jabgui/src/test/java/org/jabref/gui/fieldeditors/contextmenu/ContextActionTest.java 🧪 Tests +1/-12

Update context action tests

• Removed test shouldNotBeExecutableForMoveToFolderWhenGeneratedPathMatchesOriginal() as behavior
 was modified
• Updated test shouldExecuteMoveToDefaultDirectory() to verify call to
 moveToNextPossibleDirectory()

jabgui/src/test/java/org/jabref/gui/fieldeditors/contextmenu/ContextActionTest.java


6. jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java ✨ Enhancement +7/-0

Add arbitrary directory move support

• Added new public method moveToDirectory(Path baseDirectory) to support moving files to arbitrary
 directories
• Refactored copyOrMoveToDefaultDirectory() to delegate to new private copyOrMoveToDirectory()
 method
• Extracted common directory copy/move logic into reusable private method

jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java


7. jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java ✨ Enhancement +42/-0

Add method to retrieve all file directories

• Added new method getAllFileDirectories(FilePreferences) returning fixed-size list of all
 configured directories
• Returns directories in order: User → Library → BIB/Main (with null for unconfigured directories)
• Enables directory rotation logic by providing positional directory information
• Added import for java.util.Arrays

jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java


8. jablib/src/main/resources/l10n/JabRef_en.properties 📝 Documentation +4/-1

Update localization strings

• Updated localization string for move action to "Move file(s) to the next configured file
 directory"
• Added three new error message strings for directory configuration scenarios
• Supports improved user feedback for missing or unconfigured directories

jablib/src/main/resources/l10n/JabRef_en.properties


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. currentFile.get() can throw ✓ Resolved 📘 Rule violation ≡ Correctness
Description
moveToNextPossibleDirectory() shows error dialogs for “no directory configured” and “file not
found”, but does not return afterward, and then unconditionally calls currentFile.get(). • This
can raise a NoSuchElementException (and potentially other downstream errors) after the UI already
indicated failure, preventing graceful degradation. • The method should fail fast after these error
conditions and avoid Optional.get() without guaranteed presence.
Code

jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[R335-352]

+        if (possibleDirPaths.stream().allMatch(Objects::isNull)) {
+            dialogService.showErrorDialogAndWait(
+                    Localization.lang("No directory found"),
+                    Localization.lang("Configure a file directory to move file(s).")
+            );
+        }
+
+        Optional<Path> currentFile = linkedFile.findIn(databaseContext, preferences.getFilePreferences());
+
+        if (currentFile.isEmpty()) {
+            dialogService.showErrorDialogAndWait(
+                    Localization.lang("File not found"),
+                    Localization.lang("Could not find file '%0'.", linkedFile.getLink())
+            );
+        }
+
+        Optional<Path> destinationDir = getNextTargetPath(currentFile.get(), possibleDirPaths);
+        if (destinationDir.isEmpty()) {
Evidence
PR Compliance ID 3 requires explicit handling of null/empty/boundary cases and avoiding
silent/unsafe failures. The code displays an error when possibleDirPaths are all null and when
currentFile is empty, but then continues and dereferences currentFile via get() anyway.

Rule 3: Generic: Robust Error Handling and Edge Case Management
jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[335-352]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`moveToNextPossibleDirectory()` continues execution after showing error dialogs and then calls `currentFile.get()` even when `currentFile` is empty, which can throw and break the “graceful failure” flow.
## Issue Context
This method handles user-triggered file moves. It must robustly handle edge cases (no configured directories, file not found) without throwing after presenting an error.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[333-358]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Null parent NPE ✓ Resolved 🐞 Bug ≡ Correctness
Description
getNextTargetPath assigns currentFile.getParent() to currentFileDir and then calls
currentFileDir.startsWith(dir). • Path#getParent() may return null (e.g., root edge cases or
single-segment paths), which will cause a NullPointerException. • This can crash the move action
even when the linked file exists.
Code

jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[R381-385]

+        Path currentFileDir = currentFile.getParent();
+        for (int i = 0; i < possibleDirPaths.size(); i++) {
+            Path dir = possibleDirPaths.get(i);
+            if (dir != null && (dir.equals(currentFileDir) || currentFileDir.startsWith(dir))) {
+                currentDirIndex = i;
Evidence
The code dereferences currentFileDir without a null check, and the dereference is on the hot path
for determining the current directory index.

jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[378-388]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getNextTargetPath()` can throw `NullPointerException` because it calls `currentFileDir.startsWith(...)` even when `currentFileDir` is null.
### Issue Context
This helper method is always invoked by the new move action and can crash the GUI.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[378-407]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Multi-select move inconsistent ✓ Resolved 🐞 Bug ≡ Correctness
Description
• Single-file context actions now allow MOVE even when isGeneratedPathSameAsOriginal() is true,
which matches the new “next directory” semantics. • Multi-selection still disables MOVE when
isGeneratedPathSameAsOriginal() is true, so users get different menu availability/behavior
depending on selection count. • This is likely an oversight after renaming/repurposing MOVE to mean
rotation.
Code

jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextAction.java[R56-65]

case MOVE_FILE_TO_FOLDER,
-                         MOVE_FILE_TO_FOLDER_AND_RENAME ->
+                         MOVE_FILE_TO_FOLDER_AND_RENAME,
+                         OPEN_FILE,
+                         OPEN_FOLDER,
+                         RENAME_FILE_TO_NAME,
+                         DELETE_FILE ->
Bindings.createBooleanBinding(
   () -> !linkedFile.getFile().isOnlineLink()
-                                            && linkedFile.getFile().findIn(databaseContext, preferences.getFilePreferences()).isPresent()
-                                            && !linkedFile.isGeneratedPathSameAsOriginal(),
+                                            && linkedFile.getFile().findIn(databaseContext, preferences.getFilePreferences()).isPresent(),
   nonNullDependencies(
Evidence
ContextAction no longer checks isGeneratedPathSameAsOriginal() for MOVE, but
MultiSelectionMenuBuilder still does, creating inconsistent UI enablement for the same command.

jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextAction.java[56-65]
jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/MultiSelectionMenuBuilder.java[214-217]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Multi-selection MOVE enablement still relies on `isGeneratedPathSameAsOriginal()`, which no longer matches the updated MOVE semantics (“next configured directory”).
### Issue Context
Single-selection and multi-selection context menus now disagree on when MOVE is allowed.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/MultiSelectionMenuBuilder.java[214-217]
- jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextAction.java[56-69]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. Rotation tests create dirs ✓ Resolved 🐞 Bug ≡ Correctness
Description
• The new rotation tests call Files.createDirectories() with a path that includes the filename
(.../test.pdf), which creates a directory named test.pdf instead of a file. • This makes the
tests unrealistic and potentially passing for the wrong reason (moving a directory instead of a
file), so they don’t validate the intended file-move behavior. • The tests should create parent
directories and then Files.createFile(...), and assert Files.isRegularFile(...) at the
destination.
Code

jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelFileRotationTest.java[R172-191]

+        Path nestedFileInUser = userDir.resolve("x/y/z/test.pdf");
+        Files.createDirectories(nestedFileInUser);
+        LinkedFile linkedFile = new LinkedFile("desc", nestedFileInUser, "pdf");
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, taskExecutor, dialogService, preferences);
+
+        viewModel.moveToNextPossibleDirectory();
+
+        assertTrue(Files.exists(libDir.resolve("x/y/z/test.pdf")));
+        assertFalse(Files.exists(userDir.resolve("x/y/z/test.pdf")));
+    }
+
+    @Test
+    void moveToNextDoesNotMirrorDirectoryStructureWhenFileNotInConfiguredDirectory() throws IOException {
+        List<Path> dirs = List.of(userDir, libDir, bibDir);
+        when(databaseContext.getAllFileDirectories(any())).thenReturn(dirs);
+
+        Path nestedFile = tempDir.resolve("x/y/z/test.pdf");
+        Files.createDirectories(nestedFile);
+        LinkedFile linkedFile = new LinkedFile("desc", nestedFile, "pdf");
Evidence
The test setup constructs nestedFileInUser/nestedFile including the filename and passes that to
createDirectories, which creates directories. The production code treats any existing path as a
resolvable "file" (it only checks Files.exists), so the test may inadvertently exercise directory
moves instead of file moves.

jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelFileRotationTest.java[172-174]
jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelFileRotationTest.java[189-191]
jablib/src/main/java/org/jabref/model/entry/LinkedFile.java[261-265]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new rotation tests create directories named `test.pdf` instead of creating `test.pdf` files, so they don’t validate real file-move behavior.
### Issue Context
`Files.createDirectories(pathWithFilename)` creates the directory tree including a final directory component named like the filename.
### Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelFileRotationTest.java[167-199]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. getAllFileDirectories returns nulls ✓ Resolved 📘 Rule violation ≡ Correctness
Description
• The new public API getAllFileDirectories(...) returns a List with null elements (via
orElse(null)), pushing null-handling burdens onto all callers. • This increases the risk of
null-related defects and makes call sites more complex compared to returning Optional (or a
dedicated value type) for each directory slot.
Code

jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[R214-238]

+    public List<@Nullable Path> getAllFileDirectories(FilePreferences preferences) {
+        Optional<Path> userFileDirectory = metaData.getUserFileDirectory(preferences.getUserAndHost()).map(this::getFileDirectoryPath);
+        Optional<Path> librarySpecificFileDirectory = metaData.getLibrarySpecificFileDirectory().map(this::getFileDirectoryPath);
+
+        Optional<Path> bibOrMainFileDirectory;
+
+        // BIB file directory or main file directory (according to (global) preferences)
+        if (preferences.shouldStoreFilesRelativeToBibFile()) {
+            bibOrMainFileDirectory = getDatabasePath().map(dbPath -> {
+                Path parentPath = dbPath.getParent();
+                if (parentPath == null) {
+                    parentPath = Path.of(System.getProperty("user.dir"));
+                    LOGGER.warn("Parent path of database file {} is null. Falling back to {}.", dbPath, parentPath);
+                }
+                return parentPath.toAbsolutePath();
+            });
+        } else {
+            bibOrMainFileDirectory = preferences.getMainFileDirectory();
+        }
+
+        return Arrays.asList(
+                userFileDirectory.orElse(null),
+                librarySpecificFileDirectory.orElse(null),
+                bibOrMainFileDirectory.orElse(null)
+        );
Evidence
PR Compliance ID 13 asks to avoid returning/passing null in new public APIs and to make nullability
explicit. While @Nullable is present, the method still returns null values as part of its public
contract, which is explicitly discouraged by the rule (prefer Optional/non-null value types).

AGENTS.md
jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[214-238]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`BibDatabaseContext.getAllFileDirectories(...)` exposes nulls in a new public API by returning a list containing `null` entries.
## Issue Context
Callers must now defensively null-check list elements, and mistakes can lead to NPEs. The compliance requirement prefers avoiding null in new public method contracts and using `Optional`/explicit types instead.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[200-239]
- jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[333-406]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Creates missing base dirs ✓ Resolved 🐞 Bug ≡ Correctness
Description
• The rotation flow uses getAllFileDirectories (configured directories, may not exist) instead of
getFirstExistingFileDir (existing directories). • LinkedFileHandler will unconditionally
Files.createDirectories(targetDirectory), which can create the base directory tree for a
mistyped/nonexistent configured directory. • This is a behavior change from the default move flow
that explicitly required an existing base directory, and may surprise users or create folders in
unexpected places.
Code

jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java[R46-52]

+    public void moveToDirectory(Path baseDirectory) throws IOException {
+        copyOrMoveToDirectory(baseDirectory, true, false);
+    }
+
/// @return true if the file was copied/moved or the same file exists in the target directory
public boolean copyOrMoveToDefaultDirectory(boolean shouldMove, boolean shouldRenameToFilenamePattern) throws IOException {
Optional<Path> databaseFileDirectoryOpt = databaseContext.getFirstExistingFileDir(filePreferences);
Evidence
The old default move path relies on getFirstExistingFileDir() which filters by Files.exists. The
new rotation path passes potentially-nonexistent directories to moveToDirectory, and the handler
creates directories automatically.

jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[214-248]
jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java[51-59]
jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java[77-80]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new rotation move can create configured-but-nonexistent base directories because it selects from `getAllFileDirectories()` and `LinkedFileHandler` creates directories.
### Issue Context
This differs from the default move flow which required an existing base directory (`getFirstExistingFileDir`).
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[333-368]
- jablib/src/main/java/org/jabref/logic/externalfiles/LinkedFileHandler.java[46-80]
- jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[200-248]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

7. Malformed `` documentation ✓ Resolved 📘 Rule violation ≡ Correctness
Description
• The new documentation comment uses an HTML ordered list (`) but formats items as dash bullets (-
...) instead of ...`, resulting in malformed/generated docs. • There is also a spelling/casing
issue in a new comment (Jabref vs JabRef), reducing professionalism and clarity.
Code

jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[R203-208]

+    /// The directories are returned in the following order:
+    /// <ol>
+    /// - user-specific directory
+    /// - library-specific directory
+    /// - BIB file directory or main file directory (depending on preferences)
+    /// </ol>
Evidence
PR Compliance ID 28 requires comments/documentation to be proofread and free of malformed text. The
`` block is not valid list markup, and the misspelling in a new comment is a typographical issue.

jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[203-208]
jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[379-379]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New documentation contains malformed list markup and a minor typo (`Jabref`).
## Issue Context
Documentation comments are part of the public developer experience and should render cleanly and be professionally written.
## Fix Focus Areas
- jablib/src/main/java/org/jabref/model/database/BibDatabaseContext.java[200-208]
- jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java[379-379]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java Outdated
Comment thread jabgui/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java Outdated
Comment thread jabgui/src/main/java/org/jabref/gui/fieldeditors/contextmenu/ContextAction.java Outdated
@testlens-app

This comment has been minimized.

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Feb 8, 2026
@github-actions github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Feb 8, 2026
MOVE_FILE_TO_FOLDER_AND_RENAME,
OPEN_FILE,
OPEN_FOLDER,
RENAME_FILE_TO_NAME,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keep the binding with !linkedFile.isGeneratedPathSameAsOriginal(), for this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if you're talking about MOVE_FILE_TO_FOLDER_AND_RENAME but if you are, the method uses the new implementation underneath (moveToNextPossibleDirectory) so it faces the same problem.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A GitHub comment is directly below the line

I commented to RENAME_FILE_TO_NAME

/// The returned list has a fixed size and preserves positional meaning.
///
/// The directories are returned in the following order:
/// <ol>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this? Do you know Markdown?

Ah, you copied existing code without linking the original code - and not fixing the Markdown of the original code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After the update, you need to update getFileDirectories to use the other method and add the directories if they are available.

Note that the record will have three entries only.

  • userFileDirectory
  • librarySpecificFileDirectory
  • fallbackDirectory

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did not know how ordered lists are implemented in markdown but the unordered points rendered in a readable manner so I did not bother looking into it. Fixed it now :)

}

/// Look up all configured file directories of this database.
/// The returned list has a fixed size and preserves positional meaning.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wrong data structure!

Use a @NullMarked record with Optional<Path> to be consistent with the other code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switched to a record

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Feb 8, 2026
@github-actions github-actions Bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels May 17, 2026
@calixtus calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers component: external-files component: entry-editor and removed status: no-bot-comments labels May 17, 2026
@calixtus
calixtus dismissed stale reviews from koppor and themself May 17, 2026 16:48

Deprecated

@calixtus
calixtus requested review from InAnYan and Siedlerchr May 17, 2026 16:48
calixtus

This comment was marked as duplicate.

@calixtus calixtus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe its good to go. Awaiting second review

@ganesh-vk

Copy link
Copy Markdown
Contributor Author

Tried running to test it out but jabgui fails, is it just me? I'm on the correct JDK (Temurin 25)

> Task :jabgui:run FAILED
Error occurred during initialization of boot layer
java.lang.module.FindException: Module org.jabref not found

I had an unrelated question that I had when working on this, I understand the User directory and Library directories exist for ensuring proper file sharing in the shared SQL database feature, but when we call a directory the "Main" file directory shouldn't it always be the first choice? I'm asking because BibDatabaseContext.getFirstExistingFileDir goes from User->Lib->Main/relative to the bib file and it is used quite a lot.

@calixtus

Copy link
Copy Markdown
Member

Is main branch running on your machine?

@ganesh-vk

ganesh-vk commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

Yes the main branch is working fine

a9ffc07 seems to be the commit causing problems, everything before it works fine

@Siedlerchr

Copy link
Copy Markdown
Member

I tested both main from source and builds.jabref.org Maybe something went either wrong with the merge or on your side. try a gradlew clean and then running again

@InAnYan InAnYan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I run the code and it works very well.

I reviewed the code (but not to the full attention I wanted), and I didn't find any red flags

@Siedlerchr
Siedlerchr added this pull request to the merge queue May 17, 2026
@github-actions github-actions Bot added the status: to-be-merged PRs which are accepted and should go into the merge-queue. label May 17, 2026
Merged via the queue into JabRef:main with commit 0a967be May 17, 2026
54 checks passed
@ganesh-vk
ganesh-vk deleted the move-linked-files-directory branch May 17, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: entry-editor component: external-files status: no-bot-comments status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers status: to-be-merged PRs which are accepted and should go into the merge-queue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File folders: Move between global and user-specific

5 participants