Skip to content

Add toggle functionality for journal abbreviation lists #12912

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

Open
wants to merge 49 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
37c8d2c
feat: add toggle functionality for journal abbreviation lists
zikunz Apr 8, 2025
1a5edb3
fix: prevent unabbreviation from disabled sources
zikunz Apr 13, 2025
ed985ec
test: add coverage for unabbreviation operations
zikunz Apr 13, 2025
5828a7e
refactor: address issues raised by the bot for existing testing code
zikunz Apr 13, 2025
2eee019
fix: ensure journal abbreviation tests work with toggle feature
zikunz Apr 13, 2025
623ab21
fix: resolve issues raised by the bot regarding testing code
zikunz Apr 13, 2025
0589d14
fix: resolve more issues raised by the bot regarding testing code
zikunz Apr 13, 2025
62ccc81
fix: resolve remaining issues raised by the bot regarding testing code
zikunz Apr 13, 2025
b6a92b8
fix: resolve additional issues raised by the bot regarding testing code
zikunz Apr 13, 2025
774f5fb
fix: try to resolve all issues raised by the bot regarding testing code
zikunz Apr 13, 2025
ed58605
feat: resolve merge conflicts with journal abbreviation toggle feature
zikunz Apr 18, 2025
00b8d24
fix: resolve merge conflicts in CHANGELOG.md
zikunz Apr 18, 2025
e3b62b7
doc: update issue number for this PR
zikunz Apr 18, 2025
d3f7ff4
Merge remote-tracking branch 'upstream/main' into feature/journal-abb…
zikunz Apr 18, 2025
230c70f
fix: make changes to src/main/resources/l10n/JabRef_en.properties
zikunz Apr 19, 2025
35900b1
Merge branch 'main' into feature/journal-abbreviation-toggle
zikunz Apr 19, 2025
fd4544d
fix: correct code style violations detected by OpenRewrite
zikunz Apr 19, 2025
d9e6b20
fix: correct code style violations detected by OpenRewrite during CI
zikunz Apr 19, 2025
fe60693
fix: correct more code style violations detected by OpenRewrite durin…
zikunz Apr 19, 2025
7ba2574
fix: correct Checkstyle errors
zikunz Apr 19, 2025
8fc0d82
fix: fix some unit testing failure
zikunz Apr 19, 2025
10491f5
fix: fix three failures in LocalizationConsistencyTest
zikunz Apr 20, 2025
d1e73e0
Merge branch 'main' into feature/journal-abbreviation-toggle
zikunz Apr 20, 2025
39c822d
feat: add a null check before attempting to establish the bidirection…
zikunz Apr 20, 2025
421b08f
fix: address 7 comments in 4 files raised by trag-bot
zikunz Apr 21, 2025
305beee
fix: correct Javadoc tag '@throws' to be preceded with an empty line
zikunz Apr 21, 2025
df1553c
fix: correct JavaDoc formatting
zikunz Apr 21, 2025
a68cc84
fix: make some methods to follow the fail-fast principle
zikunz Apr 21, 2025
b1237af
fix: address the comments by trag-bot
zikunz Apr 23, 2025
09999db
Merge branch 'main' into feature/journal-abbreviation-toggle
zikunz Apr 23, 2025
2f9fc82
fix: address Checkstyle issues
zikunz Apr 23, 2025
19cf34b
fix: address additional Checkstyle issues
zikunz Apr 23, 2025
9dafa2a
fix: address the final Checkstyle issue
zikunz Apr 23, 2025
9b4a43f
fix: apply suggestions from code review
zikunz Apr 23, 2025
d555a09
doc: add JavaDoc suggested by trag-bot
zikunz Apr 23, 2025
c85ec97
fix: address comments by trag-bot
zikunz Apr 23, 2025
5160d78
fix: correct Checkstyle issues
zikunz Apr 23, 2025
ccea652
Merge branch 'main' into feature/journal-abbreviation-toggle
zikunz Apr 25, 2025
a57193f
fix: restore comment inadvertently removed earlier on
zikunz Apr 26, 2025
818436d
Merge remote-tracking branch 'upstream/main' into feature/journal-abb…
zikunz Apr 26, 2025
26fc774
fix: extract the true to be constants to improve readability
zikunz Apr 26, 2025
153c8ad
fix: put the default in JabRefCliPreferences using defaults.put(ENABL…
zikunz Apr 26, 2025
2a6604e
fix: modernize collection usage, change return and param types, updat…
zikunz Apr 26, 2025
a23d3c8
fix: address Checkstyle issues
zikunz Apr 26, 2025
d9437e2
fix: address unit testing issues
zikunz Apr 26, 2025
c15e554
feat: implement JournalAbbreviationRepositoryManager for efficient re…
zikunz Apr 26, 2025
f50883d
fix: address trag-bot comments
zikunz Apr 26, 2025
7247038
fix: address Checkstyle issues
zikunz Apr 26, 2025
19d1401
Merge branch 'main' into feature/journal-abbreviation-toggle
zikunz Apr 27, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We added a "LTWA" abbreviation feature in the "Quality > Abbreviate journal names > LTWA" menu [#12273](https://github.com/JabRef/jabref/issues/12273/)
- We added path validation to file directories in library properties dialog. [#11840](https://github.com/JabRef/jabref/issues/11840)
- We now support usage of custom CSL styles in the Open/LibreOffice integration. [#12337](https://github.com/JabRef/jabref/issues/12337)
- We added ability to toggle journal abbreviation lists (including built-in and external CSV files) on/off in preferences. [#12468](https://github.com/JabRef/jabref/pull/12468)
- We added support for citation-only CSL styles which don't specify bibliography formatting. [#12996](https://github.com/JabRef/jabref/pull/12996)

### Changed
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/jabref/gui/frame/MainMenu.java
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,12 @@ private void createMenu() {
new SeparatorMenuItem(),

factory.createSubMenu(StandardActions.ABBREVIATE,
factory.createMenuItem(StandardActions.ABBREVIATE_DEFAULT, new AbbreviateAction(StandardActions.ABBREVIATE_DEFAULT, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager)),
factory.createMenuItem(StandardActions.ABBREVIATE_DOTLESS, new AbbreviateAction(StandardActions.ABBREVIATE_DOTLESS, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager)),
factory.createMenuItem(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, new AbbreviateAction(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager)),
factory.createMenuItem(StandardActions.ABBREVIATE_LTWA, new AbbreviateAction(StandardActions.ABBREVIATE_LTWA, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager))),
factory.createMenuItem(StandardActions.ABBREVIATE_DEFAULT, new AbbreviateAction(StandardActions.ABBREVIATE_DEFAULT, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager)),
factory.createMenuItem(StandardActions.ABBREVIATE_DOTLESS, new AbbreviateAction(StandardActions.ABBREVIATE_DOTLESS, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager)),
factory.createMenuItem(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, new AbbreviateAction(StandardActions.ABBREVIATE_SHORTEST_UNIQUE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager)),
factory.createMenuItem(StandardActions.ABBREVIATE_LTWA, new AbbreviateAction(StandardActions.ABBREVIATE_LTWA, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager))),

factory.createMenuItem(StandardActions.UNABBREVIATE, new AbbreviateAction(StandardActions.UNABBREVIATE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), abbreviationRepository, taskExecutor, undoManager))
factory.createMenuItem(StandardActions.UNABBREVIATE, new AbbreviateAction(StandardActions.UNABBREVIATE, frame::getCurrentLibraryTab, dialogService, stateManager, preferences.getJournalAbbreviationPreferences(), taskExecutor, undoManager))
);

Menu lookupIdentifiers = factory.createSubMenu(StandardActions.LOOKUP_DOC_IDENTIFIER);
Expand Down
131 changes: 125 additions & 6 deletions src/main/java/org/jabref/gui/journals/AbbreviateAction.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package org.jabref.gui.journals;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

import javax.swing.undo.UndoManager;
Expand All @@ -12,13 +16,16 @@
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.actions.StandardActions;
import org.jabref.gui.undo.NamedCompound;
import org.jabref.logic.journals.Abbreviation;
import org.jabref.logic.journals.JournalAbbreviationLoader;
import org.jabref.logic.journals.JournalAbbreviationPreferences;
import org.jabref.logic.journals.JournalAbbreviationRepository;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.util.BackgroundTask;
import org.jabref.logic.util.TaskExecutor;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;

import org.slf4j.Logger;
Expand All @@ -36,7 +43,7 @@ public class AbbreviateAction extends SimpleCommand {
private final DialogService dialogService;
private final StateManager stateManager;
private final JournalAbbreviationPreferences journalAbbreviationPreferences;
private final JournalAbbreviationRepository abbreviationRepository;
private JournalAbbreviationRepository abbreviationRepository;
private final TaskExecutor taskExecutor;
private final UndoManager undoManager;

Expand All @@ -47,15 +54,13 @@ public AbbreviateAction(StandardActions action,
DialogService dialogService,
StateManager stateManager,
JournalAbbreviationPreferences abbreviationPreferences,
JournalAbbreviationRepository abbreviationRepository,
TaskExecutor taskExecutor,
UndoManager undoManager) {
this.action = action;
this.tabSupplier = tabSupplier;
this.dialogService = dialogService;
this.stateManager = stateManager;
this.journalAbbreviationPreferences = abbreviationPreferences;
this.abbreviationRepository = abbreviationRepository;
this.taskExecutor = taskExecutor;
this.undoManager = undoManager;

Expand All @@ -77,6 +82,11 @@ public AbbreviateAction(StandardActions action,

@Override
public void execute() {
if (action == StandardActions.UNABBREVIATE && !areAnyJournalSourcesEnabled()) {
dialogService.notify(Localization.lang("Cannot unabbreviate: all journal lists are disabled."));
return;
}

if ((action == StandardActions.ABBREVIATE_DEFAULT)
|| (action == StandardActions.ABBREVIATE_DOTLESS)
|| (action == StandardActions.ABBREVIATE_SHORTEST_UNIQUE)
Expand All @@ -98,6 +108,9 @@ public void execute() {
}

private String abbreviate(BibDatabaseContext databaseContext, List<BibEntry> entries) {
// Reload repository to ensure latest preferences are used
abbreviationRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences);

UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(
abbreviationRepository,
abbreviationType,
Expand All @@ -119,13 +132,77 @@ private String abbreviate(BibDatabaseContext databaseContext, List<BibEntry> ent
return Localization.lang("Abbreviated %0 journal names.", String.valueOf(count));
}

/**
* Unabbreviate journal names in entries, respecting the enabled/disabled state of sources.
* Only unabbreviates entries from enabled sources.
*/
private String unabbreviate(BibDatabaseContext databaseContext, List<BibEntry> entries) {
UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(abbreviationRepository);

List<BibEntry> filteredEntries = new ArrayList<>();

JournalAbbreviationRepository freshRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences);
Copy link
Member

Choose a reason for hiding this comment

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

We also had a question about this here - do you wish to create a new repository every time you want to call unabbreviate?
Or even more generally, should unabbreviate depend on preferences at all?


Map<String, Boolean> sourceEnabledStates = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Why do you collect a Map instead of just using freshRepository.isSourceEnabled()?

String builtInId = JournalAbbreviationRepository.BUILTIN_LIST_ID;
sourceEnabledStates.put(builtInId, journalAbbreviationPreferences.isSourceEnabled(builtInId));

for (String listPath : journalAbbreviationPreferences.getExternalJournalLists()) {
if (listPath != null && !listPath.isBlank()) {
String fileName = Path.of(listPath).getFileName().toString();
sourceEnabledStates.put(fileName, journalAbbreviationPreferences.isSourceEnabled(fileName));
}
}

var allAbbreviationsWithSources = freshRepository.getAllAbbreviationsWithSources();
Map<String, List<JournalAbbreviationRepository.AbbreviationWithSource>> textToSourceMap = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Isn't abbreviation repository is already kinda a Map? Isn't there a method for getting abbreviations/unabbreviations from there


for (var abbrWithSource : allAbbreviationsWithSources) {
Abbreviation abbr = abbrWithSource.getAbbreviation();
addToMap(textToSourceMap, abbr.getName(), abbrWithSource);
addToMap(textToSourceMap, abbr.getAbbreviation(), abbrWithSource);
addToMap(textToSourceMap, abbr.getDotlessAbbreviation(), abbrWithSource);
addToMap(textToSourceMap, abbr.getShortestUniqueAbbreviation(), abbrWithSource);
}

for (BibEntry entry : entries) {
boolean includeEntry = true;

for (Field journalField : FieldFactory.getJournalNameFields()) {
if (!entry.hasField(journalField)) {
continue;
}

String text = entry.getFieldLatexFree(journalField).orElse("");
List<JournalAbbreviationRepository.AbbreviationWithSource> possibleSources =
textToSourceMap.getOrDefault(text, List.of());
Copy link
Member

Choose a reason for hiding this comment

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

This is what I talked about in the previous comment. Doesn't abbreviation repository doesn't have some kind of get method?


if (!possibleSources.isEmpty()) {
boolean allSourcesDisabled = true;
for (var abbrWithSource : possibleSources) {
String source = abbrWithSource.getSource();
if (sourceEnabledStates.getOrDefault(source, true)) {
allSourcesDisabled = false;
break;
}
}

if (allSourcesDisabled) {
includeEntry = false;
break;
}
}
}

if (includeEntry) {
filteredEntries.add(entry);
}
}

UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(freshRepository);
NamedCompound ce = new NamedCompound(Localization.lang("Unabbreviate journal names"));
int count = entries.stream().mapToInt(entry ->
int count = filteredEntries.stream().mapToInt(entry ->
(int) FieldFactory.getJournalNameFields().stream().filter(journalField ->
undoableAbbreviator.unabbreviate(databaseContext.getDatabase(), entry, journalField, ce)).count()).sum();

if (count == 0) {
return Localization.lang("No journal names could be unabbreviated.");
}
Expand All @@ -135,4 +212,46 @@ private String unabbreviate(BibDatabaseContext databaseContext, List<BibEntry> e
tabSupplier.get().markBaseChanged();
return Localization.lang("Unabbreviated %0 journal names.", String.valueOf(count));
}

/**
* Helper method to add an abbreviation to the text-to-source map
*
* @param map The map to add to
* @param text The text to use as key
* @param abbrWithSource The abbreviation with source to add
*/
private void addToMap(Map<String, List<JournalAbbreviationRepository.AbbreviationWithSource>> map,
String text,
JournalAbbreviationRepository.AbbreviationWithSource abbrWithSource) {
if (text == null || text.isBlank()) {
return;
}

map.computeIfAbsent(text, k -> new ArrayList<>()).add(abbrWithSource);
}

/**
* Checks if any journal abbreviation source is enabled in the preferences.
* This includes both the built-in list and any external journal lists.
*
* @return true if at least one source is enabled, false if all sources are disabled
*/
private boolean areAnyJournalSourcesEnabled() {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be moved to JournalAbbreviationPreferences. This is not related to logic in AbbreviateAction, and it's quite general

boolean anySourceEnabled = journalAbbreviationPreferences.isSourceEnabled(JournalAbbreviationRepository.BUILTIN_LIST_ID);

if (!anySourceEnabled) {
for (String listPath : journalAbbreviationPreferences.getExternalJournalLists()) {
if (listPath != null && !listPath.isBlank()) {
// Just check the filename since that's what's used as the source key
String fileName = Path.of(listPath).getFileName().toString();
if (journalAbbreviationPreferences.isSourceEnabled(fileName)) {
anySourceEnabled = true;
break;
}
}
}
}

return anySourceEnabled;
}
}
15 changes: 9 additions & 6 deletions src/main/java/org/jabref/gui/journals/UndoableUnabbreviator.java
Copy link
Member

Choose a reason for hiding this comment

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

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ public UndoableUnabbreviator(JournalAbbreviationRepository journalAbbreviationRe

/**
* Unabbreviate the journal name of the given entry.
* This method respects the enabled/disabled state of journal abbreviation sources.
* If an abbreviation comes from a disabled source, it will not be unabbreviated.
*
* @param entry The entry to be treated.
* @param field The field
Expand All @@ -41,16 +43,17 @@ public boolean unabbreviate(BibDatabase database, BibEntry entry, Field field, C
if (database != null) {
text = database.resolveForStrings(text);
}

if (!journalAbbreviationRepository.isKnownName(text)) {
return false; // Cannot do anything if it is not known.
}


if (!journalAbbreviationRepository.isAbbreviatedName(text)) {
return false; // Cannot unabbreviate unabbreviated name.
}

var abbreviationOpt = journalAbbreviationRepository.get(text);
if (abbreviationOpt.isEmpty()) {
return false;
}

Abbreviation abbreviation = journalAbbreviationRepository.get(text).get();
Abbreviation abbreviation = abbreviationOpt.get();
String newText = abbreviation.getName();
entry.setField(field, newText);
ce.addEdit(new UndoableFieldChange(entry, field, origText, newText));
Expand Down
Loading
Loading