-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
base: main
Are you sure you want to change the base?
Changes from 27 commits
37c8d2c
1a5edb3
ed985ec
5828a7e
2eee019
623ab21
0589d14
62ccc81
b6a92b8
774f5fb
ed58605
00b8d24
e3b62b7
d3f7ff4
230c70f
35900b1
fd4544d
d9e6b20
fe60693
7ba2574
8fc0d82
10491f5
d1e73e0
39c822d
421b08f
305beee
df1553c
a68cc84
b1237af
09999db
2f9fc82
19cf34b
9dafa2a
9b4a43f
d555a09
c85ec97
5160d78
ccea652
a57193f
818436d
26fc774
153c8ad
2a6604e
a23d3c8
d9437e2
c15e554
f50883d
7247038
19d1401
50c3549
bd53428
9c3d8cc
0cf60ea
eb07892
c8b9b51
f58543b
3a40825
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 |
---|---|---|
@@ -1,6 +1,11 @@ | ||
package org.jabref.gui.journals; | ||
|
||
import java.nio.file.Path; | ||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.function.Supplier; | ||
|
||
import javax.swing.undo.UndoManager; | ||
|
@@ -12,13 +17,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; | ||
|
@@ -36,7 +44,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; | ||
|
||
|
@@ -47,15 +55,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; | ||
|
||
|
@@ -77,6 +83,11 @@ public AbbreviateAction(StandardActions action, | |
|
||
@Override | ||
public void execute() { | ||
if (action == StandardActions.UNABBREVIATE && !areAnyJournalSourcesEnabled()) { | ||
zikunz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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) | ||
|
@@ -98,6 +109,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, | ||
|
@@ -119,13 +133,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<>(); | ||
zikunz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
JournalAbbreviationRepository freshRepository = JournalAbbreviationLoader.loadRepository(journalAbbreviationPreferences); | ||
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. We also had a question about this here - do you wish to create a new repository every time you want to call unabbreviate? 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. I will resolve this conversation after I get approval for the latest approach stated in #12912 (comment) and #12912 (comment). :) |
||
|
||
Map<String, Boolean> sourceEnabledStates = new HashMap<>(); | ||
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. Why do you collect a 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. Oh yes, |
||
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<>(); | ||
InAnYan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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, Collections.emptyList()); | ||
zikunz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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."); | ||
} | ||
|
@@ -135,4 +213,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()) { | ||
zikunz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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() { | ||
InAnYan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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; | ||
} | ||
} |
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. I've seen that you removed line https://github.com/JabRef/jabref/pull/12912/files#diff-3fa252360fc27ea81a62af5e94c9a8263b2519ca8845cd50c55d0a001f334e41L123 -- is this class still used anywhere? 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. Yes, but this class is still being used. I have simplified the filtering logic to determine which entries to process, but the actual unabbreviation is still performed by UndoableUnabbreviator undoableAbbreviator = new UndoableUnabbreviator(freshRepository);
NamedCompound ce = new NamedCompound(Localization.lang("Unabbreviate journal names"));
int count = filteredEntries.stream().mapToInt(entry ->
(int) FieldFactory.getJournalNameFields().stream().filter(journalField ->
undoableAbbreviator.unabbreviate(databaseContext.getDatabase(), entry, journalField, ce)).count()).sum(); There are also tests that verify its functionality. This class is responsible for the actual unabbreviation and undo operations, while my changes simplified how we determine which entries to process. The class is still essential for the functionality to work correctly. May I ask whether this sounds right? |
Uh oh!
There was an error while loading. Please reload this page.