Skip to content

Conversation

@ErwanGou
Copy link

@ErwanGou ErwanGou commented Nov 7, 2025

Closes #10930

Changed or Added files

The class DirectoryGroup.java contains the new type of group. There are some tests in DirectoryGroupTest.java but most of the feature cannot be tested because it needs the WatchService.

The changes in GroupNodeViewModel.java allow directory groups to be edited or removed. The root node of the mirrored structure can be dragged.

The changes in GroupDialog.fxml, GroupDialogView.java and GroupDialogViewModel.java add the button to mirror the user's local structure within JabRef. The GroupDialogViewModelTest.java was updated to fit the new constructor and to test the new feature.

The new version of JabRef_en.properties contains the English text displayed to the user when trying to mirror its local structure.

The files DirectoryUpdateListener.java, DirectoryUpdateMonitor.java, DummyDirectoryUpdateMonitor.java and DefaultDirectoryUpdateMonitor.java implement a watcher for directories. The file JabRefGUI.java is now able to initialize this watcher.

The file GroupTreeViewModel.java can now create a directory structure. It needed new attributes in the constructor so GroupTreeViewModelTest.java has been updated.

The changes in GroupTreeView.java correct a bug : the user could use "Sort subgroups Z-A" on groups that are not sortable.

In the file CHANGELOG.md there is the description of what was changed in the code.

There are many other changes to create a DirectoryGroup from a string representation.

Next steps

There is an issue with the watcher on Windows : the user can't rename, move or delete a local directory that is not empty because it is detected as opened in an application.

I chose not to allow the user to add entries in a directory group because it is supposed to work only automatically, but due to this choice they are not sortable. About this point I think the existing implementation of sortAlphabeticallyRecursive() and sortReverseAlphabeticallyRecursive() in GroupTreeViewModel.java is a bit weird :

  • It is recursive but never checks if the subgroup is sortable so we can actually sort Directory Groups if we sort the AllEntries node.
  • Furthermore I didn't get why it is mandatory that group.canAddEntriesIn() should be true because it seems the sorted method never uses that.

Steps to test

Start JabRef and open a library, empty or not :

image

You can add a new group by clicking on the 'Add group' button. A dialog should show up. Select 'Directory structure' under the 'Collect by' title :

image

Here you can select the root folder of the structure you want to mirror by clicking on the folder icon button :

image

Once the root is set it should automatically fill the 'Root path'. It also fills the 'Name' of the group if and only if it was empty and set the hierarchical context to 'Union', which means a group contains the entries of its subgroups -it is not mandatory to use that but it is recommended because it is how local folders work. You can also write or paste the root path instead of selecting the folder but remember that the 'OK' button won't be active until a valid path and a valid name are provided. You can add a description, an icon, a color or even change the hierarchical context if you want :

image

Then click on the 'OK' button, your local structure should appear :
image

The entries created with the local PDFs should appear a little while later :

image

Now feel free to test the feature. You can either add, delete, rename or move PDFs or folders in your local directory structure, you should see the consequences on your library groups. You can also edit those groups with a right-click. Remember that if you remove a group or change its type it won't adapt to your local changes anymore. If you create an entry that has a file within the mirrored structure it should automatically be added to the respective group. The feature also works if you have multiple opened libraries.

Mandatory checks

ErwanGou and others added 25 commits November 3, 2025 15:16
…x-issue-10930

# Conflicts:
#	jabgui/src/main/java/org/jabref/gui/groups/GroupDialogView.java
#	jabgui/src/main/java/org/jabref/gui/groups/GroupDialogViewModel.java
#	jabgui/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java
#	jabgui/src/main/resources/org/jabref/gui/groups/GroupDialog.fxml
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

Hey @ErwanGou!

Thank you for contributing to JabRef! Your help is truly appreciated ❤️.

We have automatic checks in place, based on which you will soon get automated feedback if any of them are failing. We also use TragBot with custom rules that scans your changes and provides some preliminary comments, before a maintainer takes a look. TragBot is still learning, and may not always be accurate. In the "Files changed" tab, you can go through its comments and just click on "Resolve conversation" if you are sure that it is incorrect, or comment on the conversation if you are doubtful.

Please re-check our contribution guide in case of any other doubts related to our contribution workflow.

@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Nov 21, 2025
@InAnYan
Copy link
Member

InAnYan commented Nov 21, 2025

Error log:

Could not parse entry
java.io.IOException: Error in line 20: Expected { or ( but received B
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.BibtexParser.consume(BibtexParser.java:1197)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.BibtexParser.parseEntry(BibtexParser.java:712)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.BibtexParser.parseAndAddEntry(BibtexParser.java:339)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.BibtexParser.parseFileContent(BibtexParser.java:265)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.BibtexParser.parse(BibtexParser.java:183)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.BibtexParser.parseEntries(BibtexParser.java:148)
	at org.jabref.jablib/org.jabref.logic.importer.Parser.parseEntries(Parser.java:18)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.pdf.PdfVerbatimBibtexImporter.importDatabase(PdfVerbatimBibtexImporter.java:34)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.pdf.PdfMergeMetadataImporter.extractCandidatesFromPdf(PdfMergeMetadataImporter.java:103)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.pdf.PdfMergeMetadataImporter.importDatabase(PdfMergeMetadataImporter.java:82)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.pdf.PdfImporter.importDatabase(PdfImporter.java:52)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.pdf.PdfMergeMetadataImporter.importDatabase(PdfMergeMetadataImporter.java:175)
	at org.jabref.jablib/org.jabref.logic.externalfiles.ExternalFilesContentImporter.importPDFContent(ExternalFilesContentImporter.java:24)
	at org.jabref/org.jabref.gui.externalfiles.ImportHandler$1.call(ImportHandler.java:147)
	at org.jabref/org.jabref.gui.externalfiles.ImportHandler$1.call(ImportHandler.java:109)
	at org.jabref/org.jabref.gui.util.UiTaskExecutor$1.call(UiTaskExecutor.java:188)
	at javafx.graphics@25.0.1/javafx.concurrent.Task$TaskCallable.call(Task.java:1407)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:328)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:545)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:328)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1095)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:619)
	at java.base/java.lang.Thread.run(Thread.java:1447)

@github-actions github-actions bot removed the status: changes-required Pull requests that are not yet complete label Nov 22, 2025
@ErwanGou
Copy link
Author

@InAnYan
I couldn't reproduce your error but it should be fixed with commit f858e08. I tried to fix it without modifying so many files but now I really think there is no other option.

Copy link
Member

@InAnYan InAnYan left a comment

Choose a reason for hiding this comment

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

Hmm, I got this exception, but I'm not sure it's because of the PR:

java.lang.NullPointerException: Cannot invoke "javafx.collections.ListChangeListener.onChanged(javafx.collections.ListChangeListener$Change)" because "<local4>[<local6>]" is null
	at javafx.base@25.0.1/com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:327)
	at javafx.base@25.0.1/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
	at javafx.base@25.0.1/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
	at javafx.base@25.0.1/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.base@25.0.1/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.base@25.0.1/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:210)
	at javafx.base@25.0.1/javafx.collections.ModifiableObservableListBase.add(ModifiableObservableListBase.java:200)
	at org.jabref.jablib/org.jabref.model.TreeNode.addChild(TreeNode.java:465)
	at org.jabref.jablib/org.jabref.model.TreeNode.addChild(TreeNode.java:447)
	at org.jabref.jablib/org.jabref.model.groups.GroupTreeNode.addSubgroup(GroupTreeNode.java:226)
	at org.jabref/org.jabref.gui.groups.GroupNodeViewModel.addSubgroup(GroupNodeViewModel.java:329)
	at org.jabref/org.jabref.gui.groups.GroupTreeViewModel.lambda$addNewSubgroup$1(GroupTreeViewModel.java:237)
	at java.base/java.util.Optional.ifPresent(Optional.java:178)
	at org.jabref/org.jabref.gui.groups.GroupTreeViewModel.lambda$addNewSubgroup$0(GroupTreeViewModel.java:236)
	at java.base/java.util.Optional.ifPresent(Optional.java:178)
	at org.jabref/org.jabref.gui.groups.GroupTreeViewModel.addNewSubgroup(GroupTreeViewModel.java:229)
	at org.jabref/org.jabref.gui.groups.GroupTreeView$ContextAction.execute(GroupTreeView.java:761)
	at org.jabref/org.jabref.gui.actions.JabRefAction.lambda$new$1(JabRefAction.java:25)
	at org.controlsfx.controls@11.2.2/org.controlsfx.control.action.Action.handle(Action.java:423)
	at org.controlsfx.controls@11.2.2/org.controlsfx.control.action.Action.handle(Action.java:64)
	at javafx.base@25.0.1/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
	at javafx.base@25.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at javafx.base@25.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at javafx.base@25.0.1/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at javafx.base@25.0.1/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@25.0.1/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at javafx.base@25.0.1/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49)
	at javafx.base@25.0.1/javafx.event.Event.fireEvent(Event.java:199)
	at javafx.controls@25.0.1/javafx.scene.control.MenuItem.fire(MenuItem.java:459)
	at javafx.controls@25.0.1/com.sun.javafx.scene.control.ContextMenuContent$MenuItemContainer.doSelect(ContextMenuContent.java:1426)
	at javafx.controls@25.0.1/com.sun.javafx.scene.control.ContextMenuContent$MenuItemContainer.lambda$createChildren$6(ContextMenuContent.java:1379)
	at javafx.base@25.0.1/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:247)
	at javafx.base@25.0.1/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
	at javafx.base@25.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at javafx.base@25.0.1/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at javafx.base@25.0.1/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at javafx.base@25.0.1/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at javafx.base@25.0.1/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@25.0.1/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@25.0.1/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@25.0.1/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@25.0.1/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@25.0.1/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@25.0.1/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@25.0.1/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at javafx.base@25.0.1/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.base@25.0.1/javafx.event.Event.fireEvent(Event.java:199)
	at javafx.graphics@25.0.1/javafx.scene.Scene$MouseHandler.process(Scene.java:4061)
	at javafx.graphics@25.0.1/javafx.scene.Scene.processMouseEvent(Scene.java:1947)
	at javafx.graphics@25.0.1/javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2784)
	at javafx.graphics@25.0.1/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.get(GlassViewEventHandler.java:353)
	at javafx.graphics@25.0.1/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.get(GlassViewEventHandler.java:255)
	at javafx.graphics@25.0.1/com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:424)
	at javafx.graphics@25.0.1/com.sun.javafx.tk.quantum.GlassViewEventHandler.handleMouseEvent(GlassViewEventHandler.java:387)
	at javafx.graphics@25.0.1/com.sun.glass.ui.View.handleMouseEvent(View.java:573)
	at javafx.graphics@25.0.1/com.sun.glass.ui.View.notifyMouse(View.java:975)
	at javafx.graphics@25.0.1/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
	at javafx.graphics@25.0.1/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$0(GtkApplication.java:240)
	at java.base/java.lang.Thread.run(Thread.java:1447)

Then, I can't add any entry to the directory group or its subgroups.

But I confirm that when I create it, it properly determines the files. And when I add a file in the directory group via file managers, JabRef really captures it (but... I got the import dialog like 4 times, IDK why)

@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Nov 23, 2025
@InAnYan
Copy link
Member

InAnYan commented Nov 23, 2025

It seems that when I add a new entry (with or without file) it immediately deletes it

@ErwanGou
Copy link
Author

@InAnYan

  • In the file GroupNodeViewModel.java we can choose whether it is possible or not to add entries manually in a group of a given type. I didn't allow it for DirectoryGroup because I thought it was supposed to be only automatic but I can easily change that if you think it's more relevant.
  • I still need to figure out why the error you sent had shown up.
  • Regarding the import dialog -if you are talking about the dialog that asks if you want to merge entries or not-, well it is not on my purpose : I use an ImportHandler to import the PDFs and it displays that each time a single file is imported. That means if you create a DirectoryGroup from a local directory structure that contains only 1 PDF the dialog will be displayed, but if there are multiple PDFs it won't. While monitoring the file system, the WatchService raise an event for each new file, so even if you paste or move multiple PDFs at the same time the dialog will show up as many times as there are PDFs. I don't know why it was implemented like that but it seems to be the best way to import files.

@koppor
Copy link
Member

koppor commented Nov 23, 2025

Then, I can't add any entry to the directory group or its subgroups.

Which is OK, isn't it?

Requirement was

Currently: Only inbound mirroring. Use WatchService to watch for changes.

And when I add a file in the directory group via file managers, JabRef really captures it (but... I got the import dialog like 4 times, IDK why)

There should be no popup at all. JabRef should import it using "best guess". We have org.jabref.logic.bibtex.comparator.FieldValuePlausibilityComparator. There could be more. Update See #14396 for the infrastructure.

@koppor
Copy link
Member

koppor commented Nov 23, 2025

  • I use an ImportHandler to import the PDFs and it displays that each time a single file is impored.

Can this be modified using #14396?

I currently get

grafik

Maybe, this involves "hiearchy" of PDF sources in case there is no comparator. Implement a list and we can re-order during code review.

@koppor
Copy link
Member

koppor commented Nov 23, 2025

  • It should not be possible to remove subgroups
    grafik
    grafik
  • It should be possible to sort A-Z and Z-A
    grafik

@koppor
Copy link
Member

koppor commented Nov 23, 2025

Error log:

Could not parse entry
java.io.IOException: Error in line 20: Expected { or ( but received B
...
	at org.jabref.jablib/org.jabref.logic.importer.Parser.parseEntries(Parser.java:18)
	at org.jabref.jablib/org.jabref.logic.importer.fileformat.pdf.PdfVerbatimBibtexImporter.importDatabase(PdfVerbatimBibtexImporter.java:34)
	at org.jabref.jablib/

...
org.jabref.logic.importer.fileformat.pdf.PdfMergeMetadataImporter.importDatabase(PdfMergeMetadataImporter.java:175)

at org.jabref.jablib/org.jabref.logic.externalfiles.ExternalFilesContentImporter.importPDFContent(ExternalFilesContentImporter.java:24)

I think, its an issue at PdfVerbatimBibtexImporter -- we should investigate your PDFs in a seperate issue (or private issue tracker)

Maybe, the log level at this point is too high -- the PdfVerbatimBibtexImporter could be simply be called on all PDFs and the exception should just be ignored than being output.

@koppor
Copy link
Member

koppor commented Nov 23, 2025

@ErwanGou Please do not reference the issue in your commits. Since you push to the branch Julien384760:fix-issue-10930 and you have this PR, it already correlates to the PR.

When mentioning the issue in the commit, all of these appear in the timeline of the issue, which is kind of strange, because all commits together solve the issue, not each single commit:

grafik

@koppor
Copy link
Member

koppor commented Nov 23, 2025

There is an issue with the watcher on Windows : the user can't rename, move or delete a local directory which contains a folder that is not empty because it is detected as opened in an application.

I think, the importers need to be checked if they add a file lock.

Can you test if this issue persists if you do a "dummy import" of the PDF files? - Meaning: not calling any importer, but creating a simple BibEntry manually?

        BibEntry entry = new BibEntry()
                .withFiles(List.of(new LinkedFile("", "example.pdf", StandardFileType.PDF.getName())));

@koppor koppor mentioned this pull request Nov 23, 2025
1 task
@koppor
Copy link
Member

koppor commented Nov 23, 2025

I think, : should be escaped when writing and unescaped when reading

grafik

I did NOT get the exception when closing and opening the library in JabRef. But I got it when closing JabRef and re-opening JabRef again. -- OK, at another branch... -- But we need to be backwards compatible. That means, version 5.15 of JabRef should be able to open the library without any exception.

@ErwanGou
Copy link
Author

  • It should not be possible to remove subgroups
  • It should be possible to sort A-Z and Z-A

I can make it so that only the root of the mirrored directory structure can be removed but there would still be a problem with remove subgroups and sort A-Z/ Z-A. The existing implementation is as follow :

case GROUP_SUBGROUP_REMOVE ->
          group.isEditable() && group.hasSubgroups() && group.canRemove()
               || group.isRoot();
case GROUP_SUBGROUP_SORT,
     GROUP_SUBGROUP_SORT_REVERSE ->
          group.isEditable() && group.hasSubgroups() && group.canAddEntriesIn()
               || group.isRoot();

That means we can remove the subgroups of a group that can be removed and it never checks if the given subgroups are really deletable, so even if only the root is deletable the user will still be allowed to use this feature.
Regarding the sort A-Z/ Z-A there is the same problem : it is recursive but never checks if it is allowed to sort the children --that's why we can currently sort DirectoryGroups using sort A-Z on the All entries node--. Moreover the sorting method never adds entries to the given group so I don't get why it is mandatory to have group.canAddEntriesIn() there. If we remove this test we could sort DirectoryGroups without any problem.


I think, the importers need to be checked if they add a file lock.

Can you test if this issue persists if you do a "dummy import" of the PDF files? - Meaning: not calling any importer, but creating a simple BibEntry manually?

I tried that and it still locks the file directory with a dummy import. I am pretty sure it comes from that line :

directory.register(watcher, StandardWatchEventKinds.ENTRY_CREATE, StandardWatchEventKinds.ENTRY_DELETE);

When a directory is registered the system automatically locks it. I don't know if there is a solution because I've never seen an application able to pass through this issue.


But we need to be backwards compatible. That means, version 5.15 of JabRef should be able to open the library without any exception.

Do you mean 5.15 is supposed to be able to open libraries that contain DirectoryGroups ? Unknown groups throw an error so is it even possible ?
It made me think about one last thing : what is the expected behaviour if the user mirrors its local directories, save the library, closes JabRef, modify its local directories and opens the old library ? Currently it displays the old version of the structure with directories that possibly don't exist anymore --or even worse, a root that was deleted--, which could lead to bugs. Do we recreate the mirrored structure from the root --if it still exists--, even if it could also duplicate the entries depending on how it is done ? Or do we store DirectoryGroups as ExplicitGroups to avoid conflicts and keep the stored library untouched even if that means to stop adapting to local changes ?

@github-actions github-actions bot removed the status: changes-required Pull requests that are not yet complete label Nov 25, 2025
@koppor koppor added the status: waiting-for-feedback The submitter or other users need to provide more information about the issue label Nov 25, 2025
@koppor
Copy link
Member

koppor commented Nov 25, 2025

  • It should not be possible to remove subgroups
    [...]
    I can make it so that only the root of the mirrored directory structure can be removed but there would still be a problem with remove subgroups

I don't understand how NOT remove relates to "remove subgroups" issue.

Do you say you want to implement subgroup removal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first contrib status: waiting-for-feedback The submitter or other users need to provide more information about the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dynamic group mirroring the file system.

4 participants