Skip to content
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.

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

Add group dialog directory structure option #10991

Closed
wants to merge 5 commits into from

Conversation

AntonSederlin
Copy link

This is an initial and partial solution to issue #10930.

Given a directory we want to mirror the hierarchy and automatically create groups and subgroups accordingly.

To partially address this issue, we added a new radio button in the "add group dialog" that allows a user to browse a local directory, import it, and then automatically create a group with the same name, see screenshot below.

image

As of now:
If you import the directory Related_Work_Collection/Quantum_Computing, only the group Quantum_Computing will be created automatically as this is the selected directory.

Future work:
If you import the directory Related_Work_Collection/Quantum_Computing, also recursively add every potential child directory inside Quantum_Computing as subgroups.

Problems:
As we tried to tackle this issue we struggled with creating sub groups recursively as this appears to require an even deeper understanding of the project architecture and thus decided to leave this to future work.

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • 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.

- Add radio button ´Directory Structure´ in the add group form
- When selected you can browse and upload a local directory
- A group with the same name as the local directory will be aotumatically created.
- Add a validator to ensure that the browsed path is a directory.
- 4 new tests that validate the correcteness of "dirGroupFilePathValidator"
- Put an entry in CHANGELOG.md for our added functionality.
Test: findMissingLocalizationKeys() FAILED
Added: Please\ provide\ a\ valid\ directory=Please provide a valid directory
To: JabRef_en.properties
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. I miss tests for nested folders and folder addition and removal during a run of JabRef.

@koppor
Copy link
Member

koppor commented Mar 19, 2024

I tried to enter a directory without "Browse". Cannot press "OK":

image

@koppor
Copy link
Member

koppor commented Mar 19, 2024

If I add a directory, it is not reflected in the tree automatically:

tmp/blubb via ❄️  impure (shell) ❯ tree
.
└── x
    └── y

3 directories, 0 files

image

@koppor
Copy link
Member

koppor commented Mar 19, 2024

A call to "Add subgroup" is possible. That is good.

image

It is bad, however, that other groups than a directory group can be created.


Proposal: Replace "Add subgroup" menu item by "Create sub directory".

@koppor
Copy link
Member

koppor commented Mar 19, 2024

Restart of JabRef does not show any sub groups - even though there are sub directories:

image

@koppor
Copy link
Member

koppor commented Mar 19, 2024

Not sure, if it is related - but there is an aux parser exception at the beginning

2024-03-19 20:22:07 [JavaFX Application Thread] org.jabref.logic.auxparser.DefaultAuxParser.parseAuxFile()
WARN: Problem opening file: java.io.IOException: Is a directory
	at java.base/sun.nio.ch.UnixFileDispatcherImpl.read0(Native Method)
	at java.base/sun.nio.ch.UnixFileDispatcherImpl.read(UnixFileDispatcherImpl.java:51)
	at java.base/sun.nio.ch.IOUtil.readIntoNativeBuffer(IOUtil.java:340)
	at java.base/sun.nio.ch.IOUtil.read(IOUtil.java:306)
	at java.base/sun.nio.ch.IOUtil.read(IOUtil.java:283)
	at java.base/sun.nio.ch.FileChannelImpl.read(FileChannelImpl.java:234)
	at java.base/sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:74)
	at java.base/sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:103)
	at java.base/sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:329)
	at java.base/sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:372)
	at java.base/sun.nio.cs.StreamDecoder.lockedRead(StreamDecoder.java:215)
	at java.base/sun.nio.cs.StreamDecoder.read(StreamDecoder.java:169)
	at java.base/java.io.InputStreamReader.read(InputStreamReader.java:188)
	at java.base/java.io.BufferedReader.fill(BufferedReader.java:160)
	at java.base/java.io.BufferedReader.implReadLine(BufferedReader.java:370)
	at java.base/java.io.BufferedReader.readLine(BufferedReader.java:347)
	at java.base/java.io.BufferedReader.readLine(BufferedReader.java:436)
	at org.jabref@100.0.0/org.jabref.logic.auxparser.DefaultAuxParser.parseAuxFile(DefaultAuxParser.java:69)
	at org.jabref@100.0.0/org.jabref.logic.auxparser.DefaultAuxParser.parse(DefaultAuxParser.java:51)
	at org.jabref@100.0.0/org.jabref.model.groups.TexGroup.contains(TexGroup.java:67)
	at org.jabref@100.0.0/org.jabref.model.groups.AbstractGroup.isMatch(AbstractGroup.java:141)
	at org.jabref@100.0.0/org.jabref.model.search.matchers.AndMatcher.lambda$isMatch$0(AndMatcher.java:13)
	at java.base/java.util.stream.MatchOps$1MatchSink.accept(MatchOps.java:90)
	at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1685)
	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:129)
	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:527)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:513)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:230)
	at java.base/java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:196)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.allMatch(ReferencePipeline.java:637)
	at org.jabref@100.0.0/org.jabref.model.search.matchers.AndMatcher.isMatch(AndMatcher.java:13)
	at org.jabref@100.0.0/org.jabref.gui.maintable.MainTableDataModel.lambda$isMatchedByGroup$4(MainTableDataModel.java:74)
	at java.base/java.util.Optional.map(Optional.java:260)
	at org.jabref@100.0.0/org.jabref.gui.maintable.MainTableDataModel.isMatchedByGroup(MainTableDataModel.java:74)
	at org.jabref@100.0.0/org.jabref.gui.maintable.MainTableDataModel.isMatched(MainTableDataModel.java:64)
	at org.jabref@100.0.0/org.jabref.gui.maintable.MainTableDataModel.lambda$new$1(MainTableDataModel.java:53)
	at javafx.base@21.0.2/javafx.collections.transformation.FilteredList.refilter(FilteredList.java:329)
	at javafx.base@21.0.2/javafx.collections.transformation.FilteredList$1.invalidated(FilteredList.java:101)
	at javafx.base@21.0.2/javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:112)
	at javafx.base@21.0.2/javafx.beans.property.ObjectPropertyBase$Listener.invalidated(ObjectPropertyBase.java:234)
	at javafx.base@21.0.2/com.sun.javafx.binding.ExpressionHelper$SingleInvalidation.fireValueChangedEvent(ExpressionHelper.java:147)
	at javafx.base@21.0.2/com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:91)
	at javafx.base@21.0.2/javafx.beans.binding.ObjectBinding.invalidate(ObjectBinding.java:192)
	at javafx.base@21.0.2/com.sun.javafx.binding.BindingHelperObserver.invalidated(BindingHelperObserver.java:52)
	at javafx.base@21.0.2/com.sun.javafx.binding.ListExpressionHelper$Generic.notifyListeners(ListExpressionHelper.java:582)
	at javafx.base@21.0.2/com.sun.javafx.binding.ListExpressionHelper$Generic.fireValueChangedEvent(ListExpressionHelper.java:546)
	at javafx.base@21.0.2/com.sun.javafx.binding.ListExpressionHelper.fireValueChangedEvent(ListExpressionHelper.java:101)
	at javafx.base@21.0.2/javafx.beans.property.ReadOnlyListPropertyBase.fireValueChangedEvent(ReadOnlyListPropertyBase.java:94)
	at javafx.base@21.0.2/javafx.beans.property.ReadOnlyListWrapper.fireValueChangedEvent(ReadOnlyListWrapper.java:106)
	at javafx.base@21.0.2/javafx.beans.property.ListPropertyBase.markInvalid(ListPropertyBase.java:223)
	at javafx.base@21.0.2/javafx.beans.property.ListPropertyBase$Listener.invalidated(ListPropertyBase.java:338)
	at javafx.base@21.0.2/com.sun.javafx.binding.ExpressionHelper$SingleInvalidation.fireValueChangedEvent(ExpressionHelper.java:147)
	at javafx.base@21.0.2/com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:91)
	at javafx.base@21.0.2/javafx.beans.binding.ObjectBinding.invalidate(ObjectBinding.java:192)
	at javafx.base@21.0.2/com.sun.javafx.binding.BindingHelperObserver.invalidated(BindingHelperObserver.java:52)
	at javafx.base@21.0.2/com.sun.javafx.collections.MapListenerHelper$SingleInvalidation.fireValueChangedEvent(MapListenerHelper.java:123)
	at javafx.base@21.0.2/com.sun.javafx.collections.MapListenerHelper.fireValueChangedEvent(MapListenerHelper.java:70)
	at javafx.base@21.0.2/com.sun.javafx.collections.ObservableMapWrapper.callObservers(ObservableMapWrapper.java:115)
	at javafx.base@21.0.2/com.sun.javafx.collections.ObservableMapWrapper.put(ObservableMapWrapper.java:169)
	at org.jabref@100.0.0/org.jabref.gui.StateManager.setSelectedGroups(StateManager.java:120)
	at org.jabref@100.0.0/org.jabref.gui.groups.GroupTreeViewModel.lambda$onSelectedGroupChanged$6(GroupTreeViewModel.java:127)
	at java.base/java.util.Optional.ifPresent(Optional.java:178)
	at org.jabref@100.0.0/org.jabref.gui.groups.GroupTreeViewModel.onSelectedGroupChanged(GroupTreeViewModel.java:123)
	at com.tobiasdiez.easybind@2.2.1-SNAPSHOT/com.tobiasdiez.easybind.EasyBind.lambda$subscribe$1(EasyBind.java:493)
	at javafx.base@21.0.2/com.sun.javafx.binding.ListExpressionHelper$Generic.notifyListeners(ListExpressionHelper.java:586)
	at javafx.base@21.0.2/com.sun.javafx.binding.ListExpressionHelper$Generic.fireValueChangedEvent(ListExpressionHelper.java:569)
	at javafx.base@21.0.2/com.sun.javafx.binding.ListExpressionHelper.fireValueChangedEvent(ListExpressionHelper.java:107)
	at javafx.base@21.0.2/javafx.beans.property.ListPropertyBase.fireValueChangedEvent(ListPropertyBase.java:203)
	at javafx.base@21.0.2/javafx.beans.property.ListPropertyBase.lambda$new$0(ListPropertyBase.java:57)
	at javafx.base@21.0.2/com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:162)
	at javafx.base@21.0.2/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
	at javafx.base@21.0.2/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
	at javafx.base@21.0.2/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.base@21.0.2/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.base@21.0.2/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:210)
	at javafx.base@21.0.2/javafx.collections.ModifiableObservableListBase.setAll(ModifiableObservableListBase.java:102)
	at javafx.base@21.0.2/javafx.beans.binding.ListExpression.setAll(ListExpression.java:342)
	at org.jabref@100.0.0/org.jabref.gui.groups.GroupTreeView.updateSelection(GroupTreeView.java:401)
	at org.jabref@100.0.0/org.jabref.gui.util.BindingsHelper$BidirectionalListBinding.onChanged(BindingsHelper.java:265)
	at javafx.base@21.0.2/com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:162)
	at javafx.base@21.0.2/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
	at javafx.base@21.0.2/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
	at javafx.base@21.0.2/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.base@21.0.2/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.base@21.0.2/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:210)
	at javafx.controls@21.0.2/com.sun.javafx.scene.control.SelectedItemsReadOnlyObservableList.lambda$new$0(SelectedItemsReadOnlyObservableList.java:91)
	at javafx.base@21.0.2/com.sun.javafx.collections.ListListenerHelper$Generic.fireValueChangedEvent(ListListenerHelper.java:327)
	at javafx.base@21.0.2/com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:71)
	at javafx.base@21.0.2/javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:246)
	at javafx.base@21.0.2/javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.base@21.0.2/javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.base@21.0.2/javafx.collections.ObservableListBase.endChange(ObservableListBase.java:210)
	at javafx.controls@21.0.2/com.sun.javafx.scene.control.ReadOnlyUnbackedObservableList._endChange(ReadOnlyUnbackedObservableList.java:55)
	at javafx.controls@21.0.2/javafx.scene.control.MultipleSelectionModelBase$SelectedIndicesList._endChange(MultipleSelectionModelBase.java:917)
	at javafx.controls@21.0.2/javafx.scene.control.ControlUtils.updateSelectedIndices(ControlUtils.java:218)
	at javafx.controls@21.0.2/javafx.scene.control.TreeTableView$TreeTableViewArrayListSelectionModel.fireCustomSelectedCellsListChangeEvent(TreeTableView.java:3564)
	at javafx.controls@21.0.2/javafx.scene.control.TreeTableView$TreeTableViewArrayListSelectionModel.clearAndSelect(TreeTableView.java:3026)
	at javafx.controls@21.0.2/com.sun.javafx.scene.control.behavior.TableCellBehaviorBase.simpleSelect(TableCellBehaviorBase.java:222)
	at javafx.controls@21.0.2/com.sun.javafx.scene.control.behavior.TableCellBehaviorBase.doSelect(TableCellBehaviorBase.java:198)
	at javafx.controls@21.0.2/com.sun.javafx.scene.control.behavior.CellBehaviorBase.mousePressed(CellBehaviorBase.java:176)
	at javafx.controls@21.0.2/com.sun.javafx.scene.control.inputmap.InputMap.handle(InputMap.java:274)
	at javafx.base@21.0.2/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:247)
	at javafx.base@21.0.2/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
	at javafx.base@21.0.2/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at javafx.base@21.0.2/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at javafx.base@21.0.2/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base@21.0.2/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base@21.0.2/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at javafx.base@21.0.2/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.base@21.0.2/javafx.event.Event.fireEvent(Event.java:198)
	at javafx.graphics@21.0.2/javafx.scene.Scene$MouseHandler.process(Scene.java:3984)
	at javafx.graphics@21.0.2/javafx.scene.Scene.processMouseEvent(Scene.java:1890)
	at javafx.graphics@21.0.2/javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2708)
	at javafx.graphics@21.0.2/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:411)
	at javafx.graphics@21.0.2/com.sun.javafx.tk.quantum.GlassViewEventHandler$MouseEventNotification.run(GlassViewEventHandler.java:301)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
	at javafx.graphics@21.0.2/com.sun.javafx.tk.quantum.GlassViewEventHandler.lambda$handleMouseEvent$2(GlassViewEventHandler.java:450)
	at javafx.graphics@21.0.2/com.sun.javafx.tk.quantum.QuantumToolkit.runWithoutRenderLock(QuantumToolkit.java:424)
	at javafx.graphics@21.0.2/com.sun.javafx.tk.quantum.GlassViewEventHandler.handleMouseEvent(GlassViewEventHandler.java:449)
	at javafx.graphics@21.0.2/com.sun.glass.ui.View.handleMouseEvent(View.java:551)
	at javafx.graphics@21.0.2/com.sun.glass.ui.View.notifyMouse(View.java:937)
	at javafx.graphics@21.0.2/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
	at javafx.graphics@21.0.2/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$10(GtkApplication.java:263)
	at java.base/java.lang.Thread.run(Thread.java:1583)

@koppor koppor marked this pull request as draft March 19, 2024 20:23
@koppor
Copy link
Member

koppor commented Mar 21, 2024

@eggestig and @AntonSederlin May we ask whether you intend to continue on this? 😅

@eggestig
Copy link

Hi @koppor! I have not been recieving any notifications for all of your updates it seems, I apologize for the late reply. We would have loved to continue, but unfortunately our course has ended and I do believe most of us are quite busy with other courses right now. So we will unfortunately not be continuing (I would possibly be able to continue if there was a guide or some hints towards how to create subgroup(s) without needing a dialogue window).

Also for clarification, we are aware that sub groups are not created (only the top-level group is). We struggled with finding a way of calling the creation of multiple groups with group dialogue window*, as mentioned in the pull request. So we decided we would push what we could make, which was the creation of the radiobutton Directory structure and its respective content for selecting a root directory, along with extracting the name of said root directory and creating a group with said name.

For the future, if someone plans on continuing, all that is left is to figure out how to create subgroups from that single dialogue window, after which it is easy to simply create the subgroups with the selected root directory as the hierarchy recursively.

Again, I apologize for the late reply!

@koppor
Copy link
Member

koppor commented Mar 22, 2024

@eggestig Thank you for the update! In future, I will make more use of the mention feature to ensure that contributors are notified!

, all that is left is to figure out how to create subgroups from that single dialogue window

I think, one needs to implement a DirectoryUpdateMonitor. This is also needed to fix #10585. See comment at #10937 (comment).


I will close the PR and hope some other group will work on this issue.

@koppor koppor closed this Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants