-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
add toggle to disable built-in journal list #15387
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
477073d
94e6f92
15d763f
7d696cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ public class JournalAbbreviationsTabViewModel implements PreferenceTabViewModel | |
| private final SimpleBooleanProperty isEditableAndRemovable = new SimpleBooleanProperty(false); | ||
| private final SimpleBooleanProperty isAbbreviationEditableAndRemovable = new SimpleBooleanProperty(false); | ||
| private final SimpleBooleanProperty useFJournal = new SimpleBooleanProperty(true); | ||
| private final SimpleBooleanProperty useBuiltInList = new SimpleBooleanProperty(true); | ||
|
|
||
| private final DialogService dialogService; | ||
| private final TaskExecutor taskExecutor; | ||
|
|
@@ -107,6 +108,7 @@ public JournalAbbreviationsTabViewModel(JournalAbbreviationPreferences abbreviat | |
| public void setValues() { | ||
| journalFiles.clear(); | ||
|
|
||
| useBuiltInList.set(abbreviationsPreferences.shouldUseBuiltInList()); | ||
| createFileObjects(); | ||
| selectLastJournalFile(); | ||
| addBuiltInList(); | ||
|
Comment on lines
+111
to
115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 5. Ui ignores built-in toggle JournalAbbreviationsTabViewModel.setValues always calls addBuiltInList(), so the Preferences UI will still show a “JabRef built in list” pseudo-file even when “Use JabRef built-in journal abbreviation list” is unchecked. Agent Prompt
|
||
|
|
@@ -309,6 +311,7 @@ public void storeSettings() { | |
|
|
||
| abbreviationsPreferences.setExternalJournalLists(journalStringList); | ||
| abbreviationsPreferences.setUseFJournalField(useFJournal.get()); | ||
| abbreviationsPreferences.setUseBuiltInList(useBuiltInList.get()); | ||
|
|
||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
| if (shouldWriteLists) { | ||
| saveJournalAbbreviationFiles(); | ||
|
|
@@ -361,4 +364,8 @@ public SimpleBooleanProperty isFileRemovableProperty() { | |
| public SimpleBooleanProperty useFJournalProperty() { | ||
| return useFJournal; | ||
| } | ||
|
|
||
| public SimpleBooleanProperty useBuiltInListProperty() { | ||
| return useBuiltInList; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,6 +82,10 @@ | |
| </Button> | ||
| </HBox> | ||
|
|
||
| <HBox alignment="CENTER_LEFT" spacing="10.0"> | ||
| <CheckBox fx:id="useBuiltInList" text="%Use JabRef built-in journal abbreviation list"/> | ||
| </HBox> | ||
|
|
||
|
Comment on lines
+85
to
+88
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the two consecutive single child , |
||
| <HBox alignment="CENTER_LEFT" spacing="10.0"> | ||
| <CheckBox fx:id="useFJournal" text="%Use the field FJournal to store the full journal name for (un)abbreviations in the entry"/> | ||
| </HBox> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,23 +35,27 @@ public static Collection<Abbreviation> readAbbreviationsFromCsvFile(Path file) t | |||||||||||||||||||||||||||||||
| public static JournalAbbreviationRepository loadRepository(JournalAbbreviationPreferences journalAbbreviationPreferences) { | ||||||||||||||||||||||||||||||||
| JournalAbbreviationRepository repository; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Initialize with built-in list | ||||||||||||||||||||||||||||||||
| try (InputStream resourceAsStream = JournalAbbreviationRepository.class.getResourceAsStream("/journals/journal-list.mv")) { | ||||||||||||||||||||||||||||||||
| if (resourceAsStream == null) { | ||||||||||||||||||||||||||||||||
| LOGGER.warn("There is no journal-list.mv. We use a default journal list."); | ||||||||||||||||||||||||||||||||
| repository = new JournalAbbreviationRepository(); | ||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||
| Path tempDir = Files.createTempDirectory("jabref-journal"); | ||||||||||||||||||||||||||||||||
| Path tempJournalList = tempDir.resolve("journal-list.mv"); | ||||||||||||||||||||||||||||||||
| Files.copy(resourceAsStream, tempJournalList); | ||||||||||||||||||||||||||||||||
| repository = new JournalAbbreviationRepository(tempJournalList, loadLtwaRepository()); | ||||||||||||||||||||||||||||||||
| tempDir.toFile().deleteOnExit(); | ||||||||||||||||||||||||||||||||
| tempJournalList.toFile().deleteOnExit(); | ||||||||||||||||||||||||||||||||
| LOGGER.debug("Loaded journal abbreviations from {}", tempJournalList.toAbsolutePath()); | ||||||||||||||||||||||||||||||||
| if (!journalAbbreviationPreferences.shouldUseBuiltInList()) { | ||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Creating an empty MVStore on disk just to get a The no arg new JournalAbbreviationRepository() is NOT a clean substitute either, it inserts a fake "Demonstration" entry and drops LTWA support. i suggested fix by adding a constructor that takes only then removes the MVStore import, temp-file dance, and the extra |
||||||||||||||||||||||||||||||||
| LOGGER.debug("Built-in journal abbreviation list is disabled by user preference."); | ||||||||||||||||||||||||||||||||
| repository = new JournalAbbreviationRepository(); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| repository = new JournalAbbreviationRepository(); | |
| try { | |
| Path tempDir = Files.createTempDirectory("jabref-empty-journal"); | |
| Path tempJournalList = tempDir.resolve("journal-list.mv"); | |
| // Create an empty MV file to represent an empty built-in journal list | |
| Files.createFile(tempJournalList); | |
| repository = new JournalAbbreviationRepository(tempJournalList, loadLtwaRepository()); | |
| tempDir.toFile().deleteOnExit(); | |
| tempJournalList.toFile().deleteOnExit(); | |
| LOGGER.debug("Created empty journal abbreviation repository with LTWA support from {}", tempJournalList.toAbsolutePath()); | |
| } catch (IOException e) { | |
| // Preserve previous behavior as a fallback if LTWA or temp files cannot be loaded/created | |
| LOGGER.warn("Could not create empty journal abbreviation repository with LTWA; falling back to default repository.", e); | |
| repository = new JournalAbbreviationRepository(); | |
| } |
Copilot
AI
Mar 22, 2026
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.
New behavior is introduced in loadRepository based on useBuiltInList, but there is no regression test verifying that (a) disabling the built-in list results in no built-in abbreviations being applied, and (b) external lists still load and override correctly. There are already unit tests covering built-in loading and LTWA behavior, so adding a focused test case here would help prevent future regressions.
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.
2. Missing toggle behavior tests 📘 Rule violation ✓ Correctness
Core logic in org.jabref.logic was changed to support disabling the built-in journal list, but no corresponding tests were added/updated in this PR to cover the new preference-controlled behavior. This increases regression risk for repository loading and preference persistence.
Agent Prompt
## Issue description
The PR adds a new preference-controlled branch to disable loading the bundled `journal-list.mv`, but lacks test coverage.
## Issue Context
This is a behavior change in `org.jabref.logic` and should be covered by unit tests (e.g., repository is empty when the built-in list is disabled, and non-empty when enabled; preference persistence via `JabRefCliPreferences` if applicable).
## Fix Focus Areas
- jablib/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java[35-59]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Uh oh!
There was an error while loading. Please reload this page.