-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Support customized entry types in BibliographyConsistencyCheck #13799
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 all commits
94e1770
b42b7c8
274d859
b9d69c2
b2bc3a4
8128685
831ed3b
9942817
ff8006e
d453516
53b83d9
b7145af
baeff03
3a79c98
d302f7c
76394d7
3f97a75
37416ff
66b2bbf
2031f0e
ad131c6
9c24a2f
ff9a529
d6d8278
0d7e37d
1fc3810
d30b731
462657a
704885f
c321b78
79af35f
3befca7
7125b76
e580f9a
d3bfbd8
6ed613a
6cc90e2
f1adef0
6cea857
c899efa
3e6c4b1
d681cd8
5040b0c
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 |
---|---|---|
|
@@ -54,10 +54,12 @@ public class PreferencesDialogViewModel extends AbstractViewModel { | |
private final DialogService dialogService; | ||
private final GuiPreferences preferences; | ||
private final ObservableList<PreferencesTab> preferenceTabs; | ||
private final BibEntryTypesManager bibEntryTypesManager; | ||
|
||
public PreferencesDialogViewModel(DialogService dialogService, GuiPreferences preferences) { | ||
public PreferencesDialogViewModel(DialogService dialogService, GuiPreferences preferences, BibEntryTypesManager bibEntryTypesManager) { | ||
this.dialogService = dialogService; | ||
this.preferences = preferences; | ||
this.bibEntryTypesManager = bibEntryTypesManager; | ||
|
||
// This enables passing unsaved preference values from the AI tab to the "web search" tab. | ||
AiTab aiTab = new AiTab(); | ||
|
@@ -190,9 +192,9 @@ public void storeAllSettings() { | |
+ Localization.lang("You must restart JabRef for this to come into effect.")); | ||
} | ||
|
||
Injector.setModelOrService(BibEntryTypesManager.class, preferences.getCustomEntryTypesRepository()); | ||
Injector.setModelOrService(BibEntryTypesManager.class, preferences.getCustomEntryTypesRepository(bibEntryTypesManager)); | ||
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. Direct dependency injection manipulation in GUI code violates layered architecture principle. This logic should be moved to a service layer in org.jabref.logic. |
||
dialogService.notify(Localization.lang("Preferences recorded.")); | ||
} | ||
}; | ||
|
||
/** | ||
* Inserts the preference values into the Properties of the ViewModel | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
package org.jabref.logic.quality.consistency; | ||
|
||
import java.util.Collection; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.LinkedHashMap; | ||
|
@@ -17,16 +16,14 @@ | |
import org.jabref.logic.bibtex.comparator.BibEntryByFieldsComparator; | ||
import org.jabref.logic.bibtex.comparator.FieldComparatorStack; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
import org.jabref.model.database.BibDatabaseMode; | ||
import org.jabref.model.entry.BibEntry; | ||
import org.jabref.model.entry.BibEntryType; | ||
import org.jabref.model.entry.BibEntryTypesManager; | ||
import org.jabref.model.entry.field.Field; | ||
import org.jabref.model.entry.field.InternalField; | ||
import org.jabref.model.entry.field.SpecialField; | ||
import org.jabref.model.entry.field.StandardField; | ||
import org.jabref.model.entry.field.UserSpecificCommentField; | ||
import org.jabref.model.entry.types.BiblatexEntryTypeDefinitions; | ||
import org.jabref.model.entry.types.BibtexEntryTypeDefinitions; | ||
import org.jabref.model.entry.types.EntryType; | ||
|
||
import com.google.common.annotations.VisibleForTesting; | ||
|
@@ -55,6 +52,12 @@ public class BibliographyConsistencyCheck { | |
StandardField.MODIFICATIONDATE | ||
); | ||
|
||
private final BibEntryTypesManager bibEntryTypesManager; | ||
|
||
public BibliographyConsistencyCheck(org.jabref.logic.preferences.CliPreferences cliPreferences, BibEntryTypesManager bibEntryTypesManager) { | ||
this.bibEntryTypesManager = cliPreferences.getCustomEntryTypesRepository(bibEntryTypesManager); | ||
} | ||
|
||
private static Set<Field> filterExcludedFields(Collection<Field> fields) { | ||
return fields.stream() | ||
.filter(field -> !EXPLICITLY_EXCLUDED_FIELDS.contains(field)) | ||
|
@@ -73,11 +76,17 @@ private static Set<Field> filterExcludedFields(Collection<Field> fields) { | |
@VisibleForTesting | ||
List<BibEntry> filterAndSortEntriesWithFieldDifferences(Set<BibEntry> entries, Set<Field> differingFields, Set<Field> requiredFields) { | ||
return entries.stream() | ||
.filter(entry -> | ||
// This removes entries that have all differing fields set (could be confusing to the user) | ||
!Collections.disjoint(entry.getFields(), differingFields) | ||
// This ensures that all entries with missing required fields are included | ||
|| !entry.getFields().containsAll(requiredFields)) | ||
.filter(entry -> { | ||
// Include entries that are missing any required fields | ||
boolean missingRequiredField = requiredFields.stream().anyMatch(field -> !entry.hasField(field)); | ||
|
||
// Include entries that have any of the differing fields | ||
boolean hasDifferingField = differingFields.stream().anyMatch(entry::hasField); | ||
|
||
// Include all entries when there are differing fields (to show inconsistencies) | ||
// OR when missing required fields | ||
return !differingFields.isEmpty() || missingRequiredField; | ||
}) | ||
.sorted(new FieldComparatorStack<>(List.of( | ||
new BibEntryByCitationKeyComparator(), | ||
new BibEntryByFieldsComparator() | ||
|
@@ -112,12 +121,10 @@ public Result check(BibDatabaseContext bibContext, BiConsumer<Integer, Integer> | |
|
||
collectEntriesIntoMaps(bibContext, entryTypeToFieldsInAnyEntryMap, entryTypeToFieldsInAllEntriesMap, entryTypeToEntriesMap); | ||
|
||
List<BibEntryType> entryTypeDefinitions; | ||
if (bibContext.getMode() == BibDatabaseMode.BIBLATEX) { | ||
entryTypeDefinitions = BiblatexEntryTypeDefinitions.ALL; | ||
} else { | ||
entryTypeDefinitions = BibtexEntryTypeDefinitions.ALL; | ||
} | ||
List<BibEntryType> entryTypeDefinitions = bibEntryTypesManager | ||
.getAllTypes(bibContext.getMode()) | ||
.stream() | ||
.toList(); | ||
Comment on lines
+124
to
+127
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. No need to convert to a list - 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. Collection is a interface though–i need some class to implement it right? For example HashSet class implement the Set interface. 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. |
||
|
||
// Use LinkedHashMap to preserve the order of Bib(tex|latex)EntryTypeDefinitions.ALL | ||
Map<EntryType, EntryTypeResult> resultMap = new LinkedHashMap<>(); | ||
|
@@ -161,7 +168,6 @@ public Result check(BibDatabaseContext bibContext, BiConsumer<Integer, Integer> | |
} | ||
|
||
private static void collectEntriesIntoMaps(BibDatabaseContext bibContext, Map<EntryType, Set<Field>> entryTypeToFieldsInAnyEntryMap, Map<EntryType, Set<Field>> entryTypeToFieldsInAllEntriesMap, Map<EntryType, Set<BibEntry>> entryTypeToEntriesMap) { | ||
BibDatabaseMode mode = bibContext.getMode(); | ||
List<BibEntry> entries = bibContext.getEntries(); | ||
|
||
for (BibEntry entry : entries) { | ||
|
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.
The patch introduces a constructor call with parameters, but it does not ensure that null is never passed to the method, which violates the guideline that null should never be passed to a method.