Pattern-driven pair renames for directory libraries (phase 5a)

CHANGELOG.md

@@ -11,6 +11,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 
 ### Added
 
+- In directory libraries, a sidecar and its PDF are now renamed together to the configured filename pattern (Preferences > Linked files) whenever the entry is edited — e.g. changing the citation key renames both files. [#741](https://github.com/JabRef/jabref-koppor/pull/741)
 - The groups panel of a directory library now mirrors the folder structure: each subdirectory appears as a group containing the entries whose files live there (updated live as files change). [#740](https://github.com/JabRef/jabref-koppor/pull/740)
 - Directory libraries now save into their sidecar files: edits are written back automatically (debounced until typing pauses; Ctrl+S forces the write and no longer creates a `.bib`), the first edit of a PDF-only entry creates a Markdown sidecar (`X.md` with the Hayagriva data as frontmatter and the comment fields as notes body), renaming a citation key renames the YAML key, and deleting an entry removes it from its file (the file is trashed once empty, the PDF stays). Hand-written content that JabRef does not understand survives rewrites. [#739](https://github.com/JabRef/jabref-koppor/pull/739)
 - Directory libraries now stay in sync with external file changes: creating, editing, deleting, or renaming `.yml`/`.md`/`.pdf` files in the opened folder updates the open library live, and renames keep the affected entries (selection and undo history survive). [#738](https://github.com/JabRef/jabref-koppor/pull/738)
@@ -19,6 +20,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 
 ### Changed
 
+- Automatic file linking no longer links the Markdown sidecars of a directory library (Markdown files with a Hayagriva frontmatter); other Markdown files are still linked. [#741](https://github.com/JabRef/jabref-koppor/pull/741)
 - We changed the Hayagriva YAML export to keep data JabRef cannot edit (short titles, person aliases, additional identifiers) when re-exporting an imported file, to write comments, and to derive `date` from the BibTeX year/month fields. [#736](https://github.com/JabRef/jabref-koppor/pull/736)
 
 ### Fixed

docs/requirements/directory-library.md

@@ -81,4 +81,16 @@ entries or subgroups can be added to them, they cannot be dragged or edited.
 
 Needs: impl
 
+## The sidecar and its PDF follow the configured filename pattern
+`req~directory-library.pattern-rename~1`
+
+When write-back touches a single-entry sidecar, the sidecar and its equally named PDF are
+renamed together to the base name the configured filename pattern (Linked files preferences)
+generates for the entry, keeping the pair in sync. Multi-entry files have no single generating
+entry and keep their name; occupied target names and pattern failures leave the current name
+untouched. Entry file links and the catalog follow the rename; the watcher does not re-import
+the renamed files.
+
+Needs: impl
+
 <!-- markdownlint-disable-file MD022 -->

docs/requirements/files.md

@@ -23,12 +23,14 @@ As a consequence, the file is copied.
 Needs: impl
 
 ## Auto-link broken linked file
-`req~logic.externalfiles.file-transfer.auto-link~1`
+`req~logic.externalfiles.file-transfer.auto-link~2`
 
 After a file is linked to an entry, the user might move the file to another directory without JabRef, leading to broken linked file.
 
 The function `Quality -> Automatically set file links` can help user to auto-link the moved files based on the broken file name, or the entry citation key.
 
+A Markdown sidecar of a directory library (a Markdown file opening with a Hayagriva frontmatter block) is an entry's source, not an attachment, and is never auto-linked. Any other Markdown file is still linked.
+
 Needs: impl, utest
 
 ## Searching unlinked files must not block result scrolling

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

@@ -39,6 +39,13 @@ public static BooleanExpression needsSavedLocalDatabase(StateManager stateManage
         return BooleanExpression.booleanExpression(binding);
     }
 
+    /// Like [#needsSavedLocalDatabase], but also accepts a directory library: both have a
+    /// location on disk to reveal in the file explorer or open a terminal in.
+    public static BooleanExpression needsDatabaseOnDisk(StateManager stateManager) {
+        EasyBinding<Boolean> binding = EasyBind.map(stateManager.activeDatabaseProperty(), context -> context.filter(c -> c.getPathOnDisk().isPresent()).isPresent());
+        return BooleanExpression.booleanExpression(binding);
+    }
+
     public static BooleanExpression needsSharedDatabase(StateManager stateManager) {
         EasyBinding<Boolean> binding = EasyBind.map(stateManager.activeDatabaseProperty(), context -> context.filter(c -> c.getLocation() == DatabaseLocation.SHARED).isPresent());
         return BooleanExpression.booleanExpression(binding);

jabgui/src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java

@@ -237,15 +237,13 @@ public static void openFolderAndSelectFile(Path fileLink,
         executeCommand(command, absolutePath, dialogService);
     }
 
-    /// Opens a new console starting on the given file location
+    /// Opens a new console: in the given directory itself, or, given a file, in the file's
+    /// parent directory.
     ///
-    /// @param file Location the console should be opened at.
-    public static void openConsole(Path file, GuiPreferences preferences, DialogService dialogService) throws IOException {
-        if (file == null) {
-            return;
-        }
-
-        String absolutePath = file.toAbsolutePath().getParent().toString();
+    /// @param fileOrDirectory Location the console should be opened at.
+    public static void openConsole(Path fileOrDirectory, GuiPreferences preferences, DialogService dialogService) throws IOException {
+        Path absolute = fileOrDirectory.toAbsolutePath();
+        String absolutePath = Files.isDirectory(absolute) ? absolute.toString() : absolute.getParent().toString();
 
         boolean useCustomTerminal = preferences.getExternalApplicationsPreferences().useCustomTerminal();
         if (!useCustomTerminal) {

jabgui/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java

@@ -21,6 +21,7 @@
 import org.jabref.gui.frame.ExternalApplicationsPreferences;
 import org.jabref.logic.FilePreferences;
 import org.jabref.logic.bibtex.FileFieldWriter;
+import org.jabref.logic.directorylibrary.MarkdownSidecar;
 import org.jabref.logic.util.io.AutoLinkPreferences;
 import org.jabref.logic.util.io.FileFinder;
 import org.jabref.logic.util.io.FileFinders;
@@ -57,6 +58,8 @@ public List<IOException> getFileExceptions() {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(AutoSetFileLinksUtil.class);
 
+    private final MarkdownSidecar markdownSidecar = new MarkdownSidecar();
+
     private final List<Path> directories;
     private final AutoLinkPreferences autoLinkPreferences;
     private final ExternalApplicationsPreferences externalApplicationsPreferences;
@@ -78,7 +81,7 @@ private AutoSetFileLinksUtil(List<Path> directories, ExternalApplicationsPrefere
         this.brokenLinkedFileNameBasedFileFinder = FileFinders.constructBrokenLinkedFileNameBasedFileFinder();
     }
 
-    /// [impl->req~logic.externalfiles.file-transfer.auto-link~1]
+    /// [impl->req~logic.externalfiles.file-transfer.auto-link~2]
     public LinkFilesResult linkAssociatedFiles(List<BibEntry> entries, BiConsumer<List<LinkedFile>, BibEntry> onAddLinkedFile) {
         LinkFilesResult result = new LinkFilesResult();
 
@@ -239,11 +242,27 @@ public Collection<LinkedFile> findAssociatedNotLinkedFilesWithFinder(
         // Only keep associated files that are not linked
         return associatedFiles
                 .stream()
+                .filter(associatedFile -> !isMarkdownSidecar(associatedFile))
                 .filter(associatedFile -> !isFileAlreadyLinked(associatedFile, linkedFiles))
                 .map(this::buildLinkedFileFromPath)
                 .toList();
     }
 
+    /// A Markdown sidecar of a directory library (Hayagriva frontmatter, see [MarkdownSidecar])
+    /// is an entry's source, not an attachment, so it must not be auto-linked. Any other
+    /// Markdown file is still linked.
+    private boolean isMarkdownSidecar(Path file) {
+        if (!MarkdownSidecar.hasMarkdownExtension(file)) {
+            return false;
+        }
+        try {
+            return markdownSidecar.looksLikeSidecar(file);
+        } catch (IOException e) {
+            LOGGER.debug("Could not probe {} for a sidecar frontmatter", file, e);
+            return false;
+        }
+    }
+
     private boolean isBrokenLinkedFile(LinkedFile file) {
         return file.findIn(directories).isEmpty();
     }

jabgui/src/main/java/org/jabref/gui/frame/JabRefFrame.java

@@ -77,7 +77,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.jabref.gui.actions.ActionHelper.needsSavedLocalDatabase;
+import static org.jabref.gui.actions.ActionHelper.needsDatabaseOnDisk;
 
 /// Represents the inner frame of the JabRef window
 public class JabRefFrame extends BorderPane implements LibraryTabContainer, UiMessageHandler {
@@ -886,12 +886,13 @@ public OpenDatabaseFolder(DialogService dialogService, StateManager stateManager
             this.dialogService = dialogService;
             this.preferences = preferences;
             this.databaseContext = databaseContext;
-            this.executable.bind(needsSavedLocalDatabase(stateManager));
+            this.executable.bind(needsDatabaseOnDisk(stateManager));
         }
 
         @Override
         public void execute() {
-            Optional.of(databaseContext.get()).flatMap(BibDatabaseContext::getDatabasePath).ifPresent(path -> {
+            // For a directory library, reveal the library's root directory itself
+            Optional.of(databaseContext.get()).flatMap(BibDatabaseContext::getPathOnDisk).ifPresent(path -> {
                 try {
                     NativeDesktop.openFolderAndSelectFile(path, preferences.getExternalApplicationsPreferences(), dialogService);
                 } catch (IOException e) {

jabgui/src/main/java/org/jabref/gui/frame/OpenConsoleAction.java

@@ -33,7 +33,7 @@ public OpenConsoleAction(Supplier<BibDatabaseContext> databaseContext, StateMana
         this.preferences = preferences;
         this.dialogService = dialogService;
 
-        this.executable.bind(ActionHelper.needsSavedLocalDatabase(stateManager));
+        this.executable.bind(ActionHelper.needsDatabaseOnDisk(stateManager));
     }
 
     /// Using this constructor will result in executing the command on the active database.
@@ -43,12 +43,15 @@ public OpenConsoleAction(StateManager stateManager, GuiPreferences preferences,
 
     @Override
     public void execute() {
-        Optional.ofNullable(databaseContext.get()).or(stateManager::getActiveDatabase).flatMap(BibDatabaseContext::getDatabasePath).ifPresent(path -> {
-            try {
-                NativeDesktop.openConsole(path, preferences, dialogService);
-            } catch (IOException e) {
-                LOGGER.info("Could not open console", e);
-            }
-        });
+        Optional.ofNullable(databaseContext.get())
+                .or(stateManager::getActiveDatabase)
+                .flatMap(BibDatabaseContext::getPathOnDisk)
+                .ifPresent(path -> {
+                    try {
+                        NativeDesktop.openConsole(path, preferences, dialogService);
+                    } catch (IOException e) {
+                        LOGGER.info("Could not open console", e);
+                    }
+                });
     }
 }

jabgui/src/main/java/org/jabref/gui/importer/actions/OpenDirectoryLibraryAction.java

@@ -3,6 +3,8 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Optional;
+import java.util.function.Function;
 
 import javafx.event.Event;
 import javafx.event.EventHandler;
@@ -27,7 +29,9 @@
 import org.jabref.logic.util.BackgroundTask;
 import org.jabref.logic.util.DirectoryMonitor;
 import org.jabref.logic.util.TaskExecutor;
+import org.jabref.logic.util.io.FileUtil;
 import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.BibEntryTypesManager;
 import org.jabref.model.util.FileUpdateMonitor;
 
@@ -146,8 +150,10 @@ private void showLibraryTab(DirectoryLibraryScanner.ScanResult scanResult, PdfEn
         libraryTab.updateTabTitle(false);
 
         BibDatabaseContext databaseContext = scanResult.databaseContext();
+        Function<BibEntry, Optional<String>> fileNameGenerator = entry -> FileUtil.createFileNameFromPattern(
+                databaseContext.getDatabase(), entry, preferences.getFilePreferences().getFileNamePattern());
         DirectoryLibrarySynchronizer synchronizer = new DirectoryLibrarySynchronizer(
-                databaseContext, scanResult.catalog(), pdfEntryFactory, this::disposeFile,
+                databaseContext, scanResult.catalog(), pdfEntryFactory, this::disposeFile, fileNameGenerator,
                 UiTaskExecutor::runInJavaFXThread);
         databaseContext.attachDirectorySynchronizer(synchronizer);
         synchronizer.startWatching(Injector.instantiateModelOrService(DirectoryMonitor.class));

jabgui/src/test/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtilTest.java

@@ -4,6 +4,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collection;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
@@ -21,7 +22,6 @@
 import org.jabref.model.entry.types.StandardEntryType;
 
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -71,6 +71,47 @@ void findAssociatedNotLinkedFilesSuccess() throws IOException {
         assertEquals(expected, actual);
     }
 
+    /// [utest->req~logic.externalfiles.file-transfer.auto-link~2]
+    @Test
+    void plainMarkdownNextToPdfIsLinkedToo() throws IOException {
+        Files.createFile(path.getParent().resolve("CiteKey.md"));
+        when(databaseContext.getFileDirectories(any())).thenReturn(List.of(path.getParent()));
+        AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(databaseContext, externalApplicationsPreferences, filePreferences, autoLinkPrefs);
+        Collection<LinkedFile> actual = util.findAssociatedNotLinkedFiles(entry);
+        assertEquals(List.of(new LinkedFile("", Path.of("CiteKey.md"), "Markdown"), new LinkedFile("", Path.of("CiteKey.pdf"), "PDF")),
+                actual.stream().sorted(Comparator.comparing(LinkedFile::getLink)).toList());
+    }
+
+    /// [utest->req~logic.externalfiles.file-transfer.auto-link~2]
+    @Test
+    void plainMarkdownFileIsLinked(@TempDir Path tempDir) throws IOException {
+        Files.createFile(tempDir.resolve("CiteKey.md"));
+        when(databaseContext.getFileDirectories(any())).thenReturn(List.of(tempDir));
+        AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(databaseContext, externalApplicationsPreferences, filePreferences, autoLinkPrefs);
+        Collection<LinkedFile> actual = util.findAssociatedNotLinkedFiles(entry);
+        assertEquals(List.of(new LinkedFile("", Path.of("CiteKey.md"), "Markdown")), actual);
+    }
+
+    /// [utest->req~logic.externalfiles.file-transfer.auto-link~2]
+    @Test
+    void markdownSidecarIsNotLinked(@TempDir Path tempDir) throws IOException {
+        Files.writeString(tempDir.resolve("CiteKey.md"), """
+                ---
+                CiteKey:
+                    type: article
+                    title: A Test Article
+                ---
+
+                # Notes
+
+                Some notes.
+                """);
+        when(databaseContext.getFileDirectories(any())).thenReturn(List.of(tempDir));
+        AutoSetFileLinksUtil util = new AutoSetFileLinksUtil(databaseContext, externalApplicationsPreferences, filePreferences, autoLinkPrefs);
+        Collection<LinkedFile> actual = util.findAssociatedNotLinkedFiles(entry);
+        assertEquals(List.of(), actual);
+    }
+
     @Test
     void findAssociatedNotLinkedFilesForEmptySearchDir() throws IOException {
         when(databaseContext.getFileDirectories(any())).thenReturn(List.of());
@@ -181,17 +222,14 @@ void findAllAssociatedNotLinkedFilesAndNotRepeated(@TempDir Path tempDir) throws
         assertEquals(expected, Set.copyOf(matchedFiles));
     }
 
-    /// [utest->req~logic.externalfiles.file-transfer.auto-link~1]
+    /// [utest->req~logic.externalfiles.file-transfer.auto-link~2]
     @Nested
-    @DisplayName("linkAssociatedFiles")
     class linkAssociatedFiles {
 
         @Nested
-        @DisplayName("byCitationKeyOnly")
         class byCitationKeyOnly {
 
             @Nested
-            @DisplayName("configuredCitationKeyDependencyWithStart")
             class configuredCitationKeyDependencyWithStart {
 
                 /// ```
@@ -355,7 +393,6 @@ void autoLinkByCitationKeyStartMatchingTwoFilesAtSubFolderAndOneMatchABrokenLink
             }
 
             @Nested
-            @DisplayName("configuredCitationKeyDependencyWithExact")
             class configuredCitationKeyDependencyWithExact {
 
                 /// ```
@@ -462,7 +499,6 @@ void autoLinkByCitationKeyExactAtSubFolder(@TempDir Path root) throws Exception
         }
 
         @Nested
-        @DisplayName("byBrokenLinkedFileNameOnly")
         class byBrokenLinkedFileNameOnly {
             /// ```
             /// CK: WeDoNotCare
@@ -744,7 +780,6 @@ void noAutoLinkCopyFileFromSubfolderToSubfolder(@TempDir Path root) throws Excep
         }
 
         @Nested
-        @DisplayName("byCitationKeyAndBrokenLinkedFileName")
         class byCitationKeyAndBrokenLinkedFileName {
 
             /// ```

jabgui/src/test/java/org/jabref/gui/util/OpenConsoleActionTest.java

@@ -35,22 +35,22 @@ void newActionGetsCurrentDatabase() {
         OpenConsoleAction action = new OpenConsoleAction(stateManager, preferences, null);
         action.execute();
         verify(stateManager, times(1)).getActiveDatabase();
-        verify(current, times(1)).getDatabasePath();
+        verify(current, times(1)).getPathOnDisk();
     }
 
     @Test
     void newActionGetsSuppliedDatabase() {
         OpenConsoleAction action = new OpenConsoleAction(() -> other, stateManager, preferences, null);
         action.execute();
         verify(stateManager, never()).getActiveDatabase();
-        verify(other, times(1)).getDatabasePath();
+        verify(other, times(1)).getPathOnDisk();
     }
 
     @Test
     void actionDefaultsToCurrentDatabase() {
         OpenConsoleAction action = new OpenConsoleAction(() -> null, stateManager, preferences, null);
         action.execute();
         verify(stateManager, times(1)).getActiveDatabase();
-        verify(current, times(1)).getDatabasePath();
+        verify(current, times(1)).getPathOnDisk();
     }
 }