Skip to content

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.

Sign up for GitHub

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

Open
AnvitaPrasad wants to merge 4 commits into
base: main
Choose a base branch
from
Open

add toggle to disable built-in journal list #15387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
477073d
add toggle to disable built-in journal list
AnvitaPrasad Mar 22, 2026
94e6f92
fix empty repo and useFJournal init from preferences
AnvitaPrasad Mar 23, 2026
15d763f
Merge upstream/main, resolve journal prefs conflict
AnvitaPrasad Apr 10, 2026
7d696cc
Merge branch 'main' into feature-disable-builtin-journal-list
subhramit Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv

### Added

- We added an option in Journal Abbreviations preferences to disable the built-in JabRef journal abbreviation list, allowing users to rely solely on their own custom lists. [#12468](https://github.com/JabRef/jabref/issues/12468)
- We added a label to the Group dropdown in the Import Dialog. [#15567](https://github.com/JabRef/jabref/issues/15567)
- We added a related work text extractor, which finds and inserts the related work text into bib entries from references in the texts. [#9840](https://github.com/JabRef/jabref/issues/9840)
- We added a hover button on group rows to quickly add a new group or subgroup. [#12289](https://github.com/JabRef/jabref/issues/12289)
Expand Down
2 changes: 2 additions & 0 deletions jabgui/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public class JournalAbbreviationsTab extends AbstractPreferenceTabView<JournalAb

@FXML private CustomTextField searchBox;
@FXML private CheckBox useFJournal;
@FXML private CheckBox useBuiltInList;

@Inject private TaskExecutor taskExecutor;
@Inject private JournalAbbreviationRepository abbreviationRepository;
Expand Down Expand Up @@ -133,6 +134,7 @@ private void setBindings() {
filteredAbbreviations.setPredicate(abbreviation -> searchTerm.isEmpty() || abbreviation.containsCaseIndependent(searchTerm)));

useFJournal.selectedProperty().bindBidirectional(viewModel.useFJournalProperty());
useBuiltInList.selectedProperty().bindBidirectional(viewModel.useBuiltInListProperty());
}

private void setAnimations() {
Expand Down
8 changes: 8 additions & 0 deletions ...i/src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTabViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -107,6 +108,8 @@ public JournalAbbreviationsTabViewModel(JournalAbbreviationPreferences abbreviat
public void setValues() {
journalFiles.clear();

useBuiltInList.set(abbreviationsPreferences.shouldUseBuiltInList());
Comment thread
calixtus marked this conversation as resolved.
useFJournal.set(abbreviationsPreferences.shouldUseFJournalField());
createFileObjects();
selectLastJournalFile();
addBuiltInList();
Comment on lines +111 to 115
Copy link
Copy Markdown
Contributor

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Mar 22, 2026

Choose a reason for hiding this comment

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

Action required

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

Expand Down Expand Up @@ -311,6 +314,7 @@ public void storeSettings() {

abbreviationsPreferences.setExternalJournalLists(journalStringList);
abbreviationsPreferences.setUseFJournalField(useFJournal.get());
abbreviationsPreferences.setUseBuiltInList(useBuiltInList.get());

Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
if (shouldWriteLists) {
saveJournalAbbreviationFiles();
Expand Down Expand Up @@ -363,4 +367,8 @@ public SimpleBooleanProperty isFileRemovableProperty() {
public SimpleBooleanProperty useFJournalProperty() {
return useFJournal;
}

public SimpleBooleanProperty useBuiltInListProperty() {
return useBuiltInList;
}
}
4 changes: 4 additions & 0 deletions jabgui/src/main/resources/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.fxml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 24, 2026

Choose a reason for hiding this comment

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

the two consecutive single child ,
consider grouping both checkboxes in a single for ex: , since they are both behavior toggles for journal abbreviations, but this is optional

<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>
Expand Down
51 changes: 35 additions & 16 deletions jablib/src/main/java/org/jabref/logic/journals/JournalAbbreviationLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import org.jabref.logic.journals.ltwa.LtwaRepository;

import org.h2.mvstore.MVStore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -35,23 +36,41 @@ 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());
if (!journalAbbreviationPreferences.shouldUseBuiltInList()) {
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 24, 2026

Choose a reason for hiding this comment

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

Creating an empty MVStore on disk just to get a Path for the constructor is heavyweight (temp dir, MVStore write, two deleteOnExit, new import)

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 LtwaRepository (what do you think?)

then removes the MVStore import, temp-file dance, and the extra return null path.

LOGGER.debug("Built-in journal abbreviation list is disabled by user preference.");
try {
Path tempDir = Files.createTempDirectory("jabref-journal-empty");
Path emptyJournalList = tempDir.resolve("journal-list.mv");
// Write an empty MV store so the repository has no built-in entries
try (MVStore store = new MVStore.Builder()
.fileName(emptyJournalList.toAbsolutePath().toString()).open()) {
store.openMap("FullToAbbreviation");
}
repository = new JournalAbbreviationRepository(emptyJournalList, loadLtwaRepository());
Comment on lines +45 to +49
Copy link
Copy Markdown
Member

@subhramit subhramit Apr 18, 2026

Choose a reason for hiding this comment

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

Would want a second look on this by @koppor

tempDir.toFile().deleteOnExit();
tempJournalList.toFile().deleteOnExit();
LOGGER.debug("Loaded journal abbreviations from {}", tempJournalList.toAbsolutePath());
emptyJournalList.toFile().deleteOnExit();
} catch (IOException e) {
LOGGER.error("Error while creating empty journal abbreviation repository", e);
return null;
}
Comment on lines +52 to +55
Copy link
Copy Markdown
Contributor

@LoayTarek5 LoayTarek5 Apr 24, 2026

Choose a reason for hiding this comment

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

If creating the temp empty MV file fails, this returns null, the caller JabRefGUI.startup() injects the result directly into DI with Injector.setModelOrService and later passes it to LanguageServerController, a null here causes a null pointer exception.

the existing missing resource path already falls back to new JournalAbbreviationRepository() rather than returning null, this new path should do the same

or better, i think it becomes unnecessary if you amke the constructor suggestion above

} else {
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
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();
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
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;
}
Comment on lines 36 to 74
Copy link

Copilot AI Mar 22, 2026

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.

Copilot uses AI. Check for mistakes.
Comment on lines 36 to 74
Copy link
Copy Markdown
Contributor

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Mar 22, 2026

Choose a reason for hiding this comment

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

Action required

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


// Read external lists
Expand Down Expand Up @@ -90,6 +109,6 @@ private static LtwaRepository loadLtwaRepository() throws IOException {
}

public static JournalAbbreviationRepository loadBuiltInRepository() {
return loadRepository(new JournalAbbreviationPreferences(List.of(), true));
return loadRepository(new JournalAbbreviationPreferences(List.of(), true, true));
}
}
21 changes: 19 additions & 2 deletions jablib/src/main/java/org/jabref/logic/journals/JournalAbbreviationPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,28 @@ public class JournalAbbreviationPreferences {

private final ObservableList<String> externalJournalLists;
private final BooleanProperty useFJournalField;
private final BooleanProperty useBuiltInList;

public JournalAbbreviationPreferences(List<String> externalJournalLists,
boolean useFJournalField) {
boolean useFJournalField,
boolean useBuiltInList) {
this.externalJournalLists = FXCollections.observableArrayList(externalJournalLists);
this.useFJournalField = new SimpleBooleanProperty(useFJournalField);
this.useBuiltInList = new SimpleBooleanProperty(useBuiltInList);
}
Comment thread
InAnYan marked this conversation as resolved.

private JournalAbbreviationPreferences() {
this(
List.of(), // externalJournalLists
true // useFJournalField
true, // useFJournalField
true // useBuiltInList
);
}

public void setAll(JournalAbbreviationPreferences preferences) {
this.externalJournalLists.setAll(preferences.externalJournalLists);
this.useFJournalField.set(preferences.shouldUseFJournalField());
this.useBuiltInList.set(preferences.shouldUseBuiltInList());
}

public static JournalAbbreviationPreferences getDefault() {
Expand All @@ -54,4 +59,16 @@ public BooleanProperty useFJournalFieldProperty() {
public void setUseFJournalField(boolean useFJournalField) {
this.useFJournalField.set(useFJournalField);
}

public boolean shouldUseBuiltInList() {
return useBuiltInList.get();
}

public BooleanProperty useBuiltInListProperty() {
return useBuiltInList;
}

public void setUseBuiltInList(boolean useBuiltInList) {
this.useBuiltInList.set(useBuiltInList);
}
}
6 changes: 5 additions & 1 deletion jablib/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ public class JabRefCliPreferences implements CliPreferences {
// Journal
private static final String EXTERNAL_JOURNAL_LISTS = "externalJournalLists";
private static final String USE_AMS_FJOURNAL = "useAMSFJournal";
private static final String USE_BUILT_IN_JOURNAL_LIST = "useBuiltInJournalList";

// Protected terms
private static final String PROTECTED_TERMS_ENABLED_EXTERNAL = "protectedTermsEnabledExternal";
Expand Down Expand Up @@ -1051,14 +1052,17 @@ public JournalAbbreviationPreferences getJournalAbbreviationPreferences() {
putStringList(EXTERNAL_JOURNAL_LISTS, journalAbbreviationPreferences.getExternalJournalLists()));
EasyBind.listen(journalAbbreviationPreferences.useFJournalFieldProperty(),
(_, _, newValue) -> putBoolean(USE_AMS_FJOURNAL, newValue));
EasyBind.listen(journalAbbreviationPreferences.useBuiltInListProperty(),
(_, _, newValue) -> putBoolean(USE_BUILT_IN_JOURNAL_LIST, newValue));

return journalAbbreviationPreferences;
}

private JournalAbbreviationPreferences getJournalAbbreviationPreferencesFromBackingStore(JournalAbbreviationPreferences defaults) {
return new JournalAbbreviationPreferences(
convertStringToList(get(EXTERNAL_JOURNAL_LISTS, convertListToString(defaults.getExternalJournalLists()))),
getBoolean(USE_AMS_FJOURNAL, defaults.useFJournalFieldProperty().get()));
getBoolean(USE_AMS_FJOURNAL, defaults.useFJournalFieldProperty().get()),
getBoolean(USE_BUILT_IN_JOURNAL_LIST, defaults.useBuiltInListProperty().get()));
}
// endregion

Expand Down
1 change: 1 addition & 0 deletions jablib/src/main/resources/l10n/JabRef_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2848,6 +2848,7 @@ A\ backup\ file\ for\ '%0'\ was\ found\ at\ [%1]=A backup file for '%0' was foun
Do\ you\ want\ to\ recover\ the\ library\ from\ the\ backup\ file?=Do you want to recover the library from the backup file?
This\ could\ indicate\ that\ JabRef\ did\ not\ shut\ down\ cleanly\ last\ time\ the\ file\ was\ used.=This could indicate that JabRef did not shut down cleanly last time the file was used.

Use\ JabRef\ built-in\ journal\ abbreviation\ list=Use JabRef built-in journal abbreviation list
Use\ the\ field\ FJournal\ to\ store\ the\ full\ journal\ name\ for\ (un)abbreviations\ in\ the\ entry=Use the field FJournal to store the full journal name for (un)abbreviations in the entry

Library\ to\ import\ into=Library to import into
Expand Down
Loading