add toggle to disable built-in journal list#15387
Open
AnvitaPrasad wants to merge 4 commits intoJabRef:mainfrom
Open
add toggle to disable built-in journal list#15387AnvitaPrasad wants to merge 4 commits intoJabRef:mainfrom
AnvitaPrasad wants to merge 4 commits intoJabRef:mainfrom
Conversation
Contributor
Review Summary by QodoAdd toggle to disable built-in journal abbreviation list
WalkthroughsDescription• Add checkbox to disable built-in journal abbreviation list • Preference persists across application restarts • Users can rely exclusively on custom journal lists • Wired through preferences, loader, and UI components Diagramflowchart LR
UI["UI Checkbox<br/>useBuiltInList"]
VM["ViewModel Property<br/>useBuiltInListProperty"]
PREFS["Preferences Storage<br/>useBuiltInList"]
LOADER["Journal Loader<br/>shouldUseBuiltInList"]
REPO["Repository<br/>Empty or Built-in"]
UI -- "bidirectional<br/>binding" --> VM
VM -- "store/load" --> PREFS
PREFS -- "check preference" --> LOADER
LOADER -- "conditional<br/>initialization" --> REPO
File Changes1. jabgui/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java
|
Contributor
Code Review by Qodo
1. JournalAbbreviationPreferences boolean constructor params
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a user-facing preference toggle to disable JabRef’s bundled journal abbreviation list, allowing users to rely only on external/custom lists.
Changes:
- Introduces a persisted
useBuiltInListjournal-abbreviation preference (default: enabled). - Wires the toggle into GUI preferences (FXML + tab + view model) and repository loading.
- Documents the new preference in the changelog and adds an i18n string.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
jablib/src/main/resources/l10n/JabRef_en.properties |
Adds localized label for the new checkbox. |
jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java |
Persists the new useBuiltInJournalList preference and binds it into JournalAbbreviationPreferences. |
jablib/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java |
Extends preferences model with a useBuiltInList property. |
jablib/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java |
Applies the preference when building the abbreviation repository. |
jabgui/src/main/resources/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.fxml |
Adds the new checkbox to the preferences UI. |
jabgui/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java |
Stores/loads the new toggle and updates repository after saving preferences. |
jabgui/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java |
Binds the new checkbox to the view model property. |
CHANGELOG.md |
Announces the new preference in the “Added” section. |
Comments suppressed due to low confidence (1)
jabgui/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java:115
setValues()always callsaddBuiltInList(), even when the newuseBuiltInListpreference is false. This will still add a “JabRef built in list” entry in the UI (and currently shows whateverjournalAbbreviationRepository.getAllLoaded()returns), which is confusing when the user explicitly disabled the built-in list.
Suggestion: only call addBuiltInList() (and show the built-in list entry) if useBuiltInList is enabled.
public void setValues() {
journalFiles.clear();
useBuiltInList.set(abbreviationsPreferences.shouldUseBuiltInList());
createFileObjects();
selectLastJournalFile();
addBuiltInList();
}
| LOGGER.debug("Loaded journal abbreviations from {}", tempJournalList.toAbsolutePath()); | ||
| if (!journalAbbreviationPreferences.shouldUseBuiltInList()) { | ||
| LOGGER.debug("Built-in journal abbreviation list is disabled by user preference."); | ||
| repository = new JournalAbbreviationRepository(); |
There was a problem hiding this comment.
When the built-in list is disabled, this uses new JournalAbbreviationRepository(), but that constructor populates a hardcoded "Demonstration" abbreviation and an empty LtwaRepository. This means the repository is not actually empty (contradicting the PR description) and it also disables LTWA-based abbreviation mode.
Consider introducing an explicit “empty repository” constructor/factory (no demo data) and decide whether LTWA should still be loaded independently of the built-in journal list toggle.
Suggested change
| 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(); | |
| } |
Comment on lines
35
to
59
| 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()) { | ||
| LOGGER.debug("Built-in journal abbreviation list is disabled by user preference."); | ||
| repository = new JournalAbbreviationRepository(); | ||
| } else { | ||
| 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()); | ||
| } | ||
| } catch (IOException e) { | ||
| LOGGER.error("Error while loading journal abbreviation repository", e); | ||
| return null; | ||
| } | ||
| } catch (IOException e) { | ||
| LOGGER.error("Error while loading journal abbreviation repository", e); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
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.
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
35
to
59
| 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()) { | ||
| LOGGER.debug("Built-in journal abbreviation list is disabled by user preference."); | ||
| repository = new JournalAbbreviationRepository(); | ||
| } else { | ||
| 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()); | ||
| } | ||
| } catch (IOException e) { | ||
| LOGGER.error("Error while loading journal abbreviation repository", e); | ||
| return null; | ||
| } | ||
| } catch (IOException e) { | ||
| LOGGER.error("Error while loading journal abbreviation repository", e); | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
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
Comment on lines
+111
to
114
| useBuiltInList.set(abbreviationsPreferences.shouldUseBuiltInList()); | ||
| createFileObjects(); | ||
| selectLastJournalFile(); | ||
| addBuiltInList(); |
Contributor
There was a problem hiding this comment.
5. Ui ignores built-in toggle 🐞 Bug ✓ Correctness
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
### Issue description
The Preferences UI always adds a built-in list entry regardless of the new `useBuiltInList` checkbox.
### Issue Context
`setValues()` calls `addBuiltInList()` unconditionally, and `addBuiltInList()` unconditionally appends a pseudo built-in file.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java[107-147]
### Recommended fix
- In `setValues()`, call `addBuiltInList()` only if `useBuiltInList.get()` is true.
- Also consider removing an existing built-in pseudo file from `journalFiles` when the user unchecks the box (so the list updates within the dialog, not only after restart).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
github-actions
Bot
added
the
status: changes-required
This comment has been minimized.
This comment has been minimized.
✅ All tests passed ✅🏷️ Commit: 94e6f92 Learn more about TestLens at testlens.app. |
github-actions
Bot
added
status: no-bot-comments
status: changes-required
Contributor
|
Your pull request conflicts with the target branch. Please merge with your code. For a step-by-step guide to resolve merge conflicts, see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-using-the-command-line. |
Member
|
No progress for 2 weeks, closing |
Contributor
|
This pull requests was closed without merging. You have been unassigned from the respective issue #12468. In case you closed the PR for yourself, you can re-open it. Please also check After submission of a pull request in CONTRIBUTING.md. |
Contributor
Author
|
@Siedlerchr, sorry for the delay! I’ve merged the latest upstream/main and resolved the conflicts. It’s up to date now. Could you please reopen the PR when you get a chance? |
Contributor
Review Summary by QodoAdd toggle to disable built-in journal abbreviation list
WalkthroughsDescription• Add checkbox to disable built-in journal abbreviation list • Users can now rely exclusively on custom journal lists • Preference persists across application restarts • Empty repository created when built-in list disabled Diagramflowchart LR
A["User Preference"] -->|useBuiltInList| B["JournalAbbreviationPreferences"]
B -->|shouldUseBuiltInList| C["JournalAbbreviationLoader"]
C -->|true| D["Load Built-in List"]
C -->|false| E["Create Empty Repository"]
D --> F["JournalAbbreviationRepository"]
E --> F
F -->|External Lists| G["Combined Abbreviations"]
File Changes1. jabgui/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java
|
Contributor
Code Review by Qodo
|
github-actions
Bot
added
status: no-bot-comments
status: changes-required
subhramit
reviewed
Apr 18, 2026
Comment on lines
+45
to
+49
| try (MVStore store = new MVStore.Builder() | ||
| .fileName(emptyJournalList.toAbsolutePath().toString()).open()) { | ||
| store.openMap("FullToAbbreviation"); | ||
| } | ||
| repository = new JournalAbbreviationRepository(emptyJournalList, loadLtwaRepository()); |
github-actions
Bot
added
status: no-bot-comments
status: changes-required
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues and pull requests
Closes #12468
PR Description
This PR adds an option in the Journal Abbreviations preferences to disable the built-in JabRef journal abbreviation list. When unchecked, JabRef will not load its bundled list, allowing users to rely exclusively on their own custom lists.
The change is intentionally minimal, a single checkbox controls a new
useBuiltInListpreference that is persisted across restarts. When disabled, the loader skips the built-injournal-list.mvand initializes an empty repository instead.Steps to test
To reproduce the problem on
main:mainbranch.To reproduce the fix on the feature branch:
feature-disable-builtin-journal-listbranch and start JabRef.Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)