-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix handling of relative-file storage and auto linking #11492
Conversation
Pull request was converted to draft
Path absolutePath = parentPath.toAbsolutePath(); | ||
if (!fileDirs.contains(absolutePath)) { | ||
fileDirs.add(absolutePath); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we can make a set out of this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not useful, we need to keep the order and we only have like 3 possible values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sortedset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sortedset
We need to keep the order.
Sequenced Collection Set is the "right" data type. It checks for containment at each insert. With my approach, only one check needs to be made.
Moreover, we have at most two elements to check. We use an ArrayList where the "forEach" is faster than with a LinkedList.
Changing the return type to a sequencedcollection results in much code changes. Using it internally and then converting to a list results in more CPU cycles than "just" checking two elements.
I can rewrite to return SequencedCollection, write an ADR, add Java comments, link to this text or just leave it.
What should I do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I just switched to LinkedHashSet
. This also ensures that the second path is checked for uniqueness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was just an idea, the code just smelled like a set. no strong opinion on this.
The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build. |
* upstream/main: Fix NPE when saving preferences (JabRef#11509) Switch to stream-based loading (JabRef#11479) Save unlinked local files dialog prefs (JabRef#11493) Add minimal support for biblatex data annotations (JabRef#11506) Fix handling of relative-file storage and auto linking (JabRef#11492) New Crowdin updates (JabRef#11504) Add missing issue numbers CSL4LibreOffice - A [GSoC '24] (JabRef#11477) Bump src/main/resources/csl-styles from `b2be5ae` to `fd6cb3e` (JabRef#11501) Bump gittools/actions from 1.1.1 to 1.2.0 (JabRef#11500) Bump com.kohlschutter.junixsocket:junixsocket-core from 2.9.1 to 2.10.0 (JabRef#11498) Bump commons-logging:commons-logging from 1.3.2 to 1.3.3 (JabRef#11499) Bump org.jsoup:jsoup from 1.17.2 to 1.18.1 (JabRef#11497) Bump com.kohlschutter.junixsocket:junixsocket-mysql from 2.9.1 to 2.10.0 (JabRef#11496) Bump org.openrewrite.recipe:rewrite-recipe-bom from 2.14.0 to 2.15.0 (JabRef#11495) FAQ updates (JabRef#11486) Update Gradle Wrapper from 8.8 to 8.9. Fix Chocolate.bib (JabRef#11491) # Conflicts: # src/main/java/org/jabref/gui/openoffice/OOBibBase.java # src/main/java/org/jabref/gui/openoffice/OpenOfficePanel.java # src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java # src/main/java/org/jabref/gui/openoffice/StyleSelectDialogViewModel.java # src/main/java/org/jabref/preferences/JabRefPreferences.java
Fixes https://github.com/JabRef/jabref-issue-melting-pot/issues/479
Follow-up to #9113
Mandatory checks
CHANGELOG.md
described in a way that is understandable for the average user (if applicable)