Skip to content

Commit 5069894

Browse files
kopporcalixtusSiedlerchr
authored
Fix citation key table (#13151)
* Fix citation key table * Use for-each instead of streams Co-authored-by: Carl Christian Snethlage <[email protected]> Co-authored-by: Christoph <[email protected]> * Fix table rendering * Add sorting of entries by citation key (instead of number of fields) * Adapt test * fix l10n --------- Co-authored-by: Carl Christian Snethlage <[email protected]> Co-authored-by: Christoph <[email protected]>
1 parent 61fb260 commit 5069894

File tree

6 files changed

+166
-74
lines changed

6 files changed

+166
-74
lines changed

‎jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByCitationKeyComparator.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.jabref.logic.bibtex.comparator;
2+
3+
import java.util.Comparator;
4+
5+
import org.jabref.model.entry.BibEntry;
6+
7+
public class BibEntryByCitationKeyComparator implements Comparator<BibEntry> {
8+
@Override
9+
public int compare(BibEntry e1, BibEntry e2) {
10+
boolean e1HasCitationKey = e1.hasCitationKey();
11+
boolean e2HasCitationKey = e2.hasCitationKey();
12+
13+
if (!e1HasCitationKey && !e2HasCitationKey) {
14+
return 0;
15+
}
16+
17+
if (e1HasCitationKey && !e2HasCitationKey) {
18+
return -1;
19+
}
20+
21+
if (!e1HasCitationKey && e2HasCitationKey) {
22+
return 1;
23+
}
24+
25+
assert e1HasCitationKey && e2HasCitationKey;
26+
27+
return e1.getCitationKey().get().compareTo(e2.getCitationKey().get());
28+
}
29+
}

‎jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByFieldsComparator.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package org.jabref.logic.bibtex.comparator;
2+
3+
import java.util.Comparator;
4+
import java.util.Iterator;
5+
6+
import org.jabref.model.entry.BibEntry;
7+
import org.jabref.model.entry.field.Field;
8+
9+
/**
10+
* Sorts entries by the number of fields and then by the field names.
11+
*/
12+
public class BibEntryByFieldsComparator implements Comparator<BibEntry> {
13+
@Override
14+
public int compare(BibEntry e1, BibEntry e2) {
15+
int sizeComparison = e1.getFields().size() - e2.getFields().size();
16+
if (sizeComparison != 0) {
17+
return sizeComparison;
18+
}
19+
Iterator<String> it1 = e1.getFields().stream().map(Field::getName).sorted().iterator();
20+
Iterator<String> it2 = e2.getFields().stream().map(Field::getName).sorted().iterator();
21+
while (it1.hasNext() && it2.hasNext()) {
22+
int fieldComparison = it1.next().compareTo(it2.next());
23+
if (fieldComparison != 0) {
24+
return fieldComparison;
25+
}
26+
}
27+
assert !it1.hasNext() && !it2.hasNext();
28+
return 0;
29+
}
30+
}

‎jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java‎

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
import java.util.Comparator;
55
import java.util.HashMap;
66
import java.util.HashSet;
7-
import java.util.Iterator;
87
import java.util.List;
98
import java.util.Map;
109
import java.util.SequencedCollection;
1110
import java.util.Set;
1211

12+
import org.jabref.logic.bibtex.comparator.BibEntryByCitationKeyComparator;
13+
import org.jabref.logic.bibtex.comparator.BibEntryByFieldsComparator;
14+
import org.jabref.logic.bibtex.comparator.FieldComparatorStack;
1315
import org.jabref.model.entry.BibEntry;
1416
import org.jabref.model.entry.field.Field;
1517
import org.jabref.model.entry.types.EntryType;
@@ -55,38 +57,23 @@ public Result check(List<BibEntry> entries) {
5557
return;
5658
}
5759

58-
List<BibEntry> sortedEntries = entryTypeToEntriesMap
60+
List<Comparator<BibEntry>> comparators = List.of(
61+
new BibEntryByCitationKeyComparator(),
62+
new BibEntryByFieldsComparator());
63+
FieldComparatorStack<BibEntry> comparatorStack = new FieldComparatorStack<>(comparators);
64+
65+
List<BibEntry> differingEntries = entryTypeToEntriesMap
5966
.get(entryType).stream()
6067
.filter(entry -> !entry.getFields().equals(commonFields))
61-
.sorted(getBibEntryComparator()).toList();
62-
resultMap.put(entryType, new EntryTypeResult(uniqueFields, sortedEntries));
68+
.sorted(comparatorStack)
69+
.toList();
70+
71+
resultMap.put(entryType, new EntryTypeResult(uniqueFields, differingEntries));
6372
});
6473

6574
return new Result(resultMap);
6675
}
6776

68-
/**
69-
* Sorts entries by the number of fields and then by the field names.
70-
*/
71-
private static Comparator<BibEntry> getBibEntryComparator() {
72-
return (e1, e2) -> {
73-
int sizeComparison = e1.getFields().size() - e2.getFields().size();
74-
if (sizeComparison != 0) {
75-
return sizeComparison;
76-
}
77-
Iterator<String> it1 = e1.getFields().stream().map(Field::getName).sorted().iterator();
78-
Iterator<String> it2 = e2.getFields().stream().map(Field::getName).sorted().iterator();
79-
while (it1.hasNext() && it2.hasNext()) {
80-
int fieldComparison = it1.next().compareTo(it2.next());
81-
if (fieldComparison != 0) {
82-
return fieldComparison;
83-
}
84-
}
85-
assert !it1.hasNext() && !it2.hasNext();
86-
return 0;
87-
};
88-
}
89-
9077
private static void collectEntriesIntoMaps(List<BibEntry> entries, Map<EntryType, Set<Field>> entryTypeToFieldsInAnyEntryMap, Map<EntryType, Set<Field>> entryTypeToFieldsInAllEntriesMap, Map<EntryType, Set<BibEntry>> entryTypeToEntriesMap) {
9178
entries.forEach(entry -> {
9279
EntryType entryType = entry.getType();

‎jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriter.java‎

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.io.IOException;
44
import java.io.Writer;
55
import java.util.ArrayList;
6+
import java.util.Collections;
67
import java.util.List;
78
import java.util.Set;
89
import java.util.StringJoiner;
@@ -53,46 +54,43 @@ public void writeFindings() throws IOException {
5354
super.writeFindings();
5455

5556
if (!isPorcelain) {
57+
int widthSymbol = Localization.lang("Symbol").length();
58+
int widthMeaning = Collections.max(List.of(
59+
Localization.lang("Meaning").length(),
60+
Localization.lang("required field is present").length(),
61+
Localization.lang("optional field is present").length(),
62+
Localization.lang("unknown field is present").length(),
63+
Localization.lang("field is absent").length()
64+
));
65+
5666
writer.write("\n");
57-
writer.write("%s | %s\n".formatted(REQUIRED_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("required field is present")));
58-
writer.write("%s | %s\n".formatted(OPTIONAL_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("optional field is present")));
59-
writer.write("%s | %s\n".formatted(UNKNOWN_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("unknown field is present")));
60-
writer.write("%s | %s\n".formatted(UNSET_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("field is absent")));
67+
writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(Localization.lang("Symbol"), Localization.lang("Meaning")));
68+
writer.write(("| " + "-".repeat(widthSymbol) + " | " + "-".repeat(widthMeaning) + " |\n").formatted("--", "--"));
69+
writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(REQUIRED_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("required field is present")));
70+
writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(OPTIONAL_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("optional field is present")));
71+
writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(UNKNOWN_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("unknown field is present")));
72+
writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(UNSET_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("field is absent")));
6173
}
6274
}
6375

6476
private void initializeColumnWidths() {
6577
columnWidths = new ArrayList<>(columnNames.size());
6678

67-
Integer max = getColumnWidthOfEntryTypes();
68-
columnWidths.add(max);
79+
int entryTypeWidth = "entry type".length();
80+
int citationKeyWidth = "citation key".length();
6981

70-
max = getColumnWidthOfCitationKeys(max);
71-
columnWidths.add(max);
82+
for (var keysAndValue : result.entryTypeToResultMap().entrySet()) {
83+
entryTypeWidth = Math.max(entryTypeWidth, keysAndValue.getKey().getDisplayName().length());
84+
for (var entry : keysAndValue.getValue().sortedEntries()) {
85+
citationKeyWidth = Math.max(citationKeyWidth, entry.getCitationKey().orElse("").length());
86+
}
87+
}
7288

89+
columnWidths.add(entryTypeWidth);
90+
columnWidths.add(citationKeyWidth);
7391
columnWidths.addAll(columnNames.stream().skip(2).map(String::length).toList());
7492
}
7593

76-
private Integer getColumnWidthOfEntryTypes() {
77-
int max = result.entryTypeToResultMap().keySet()
78-
.stream()
79-
.map(entryType -> entryType.getDisplayName().length())
80-
.max(Integer::compareTo)
81-
.get();
82-
max = Math.max(max, "entry type".length());
83-
return max;
84-
}
85-
86-
private Integer getColumnWidthOfCitationKeys(Integer max) {
87-
result.entryTypeToResultMap().values()
88-
.stream()
89-
.flatMap(entryTypeResult -> entryTypeResult.sortedEntries().stream())
90-
.map(entry -> entry.getCitationKey().orElse("").length())
91-
.max(Integer::compareTo)
92-
.get();
93-
return Math.max(max, "citation key".length());
94-
}
95-
9694
@Override
9795
protected void writeBibEntry(BibEntry bibEntry, String entryType, Set<Field> requiredFields, Set<Field> optionalFields) throws IOException {
9896
List<String> theRecord = getFindingsAsList(bibEntry, entryType, requiredFields, optionalFields);

‎jablib/src/main/resources/l10n/JabRef_en.properties‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,9 @@ Consistency\ check\ completed=Consistency check completed
10251025
Check\ consistency=Check consistency
10261026
Consistency\ check\ failed.=Consistency check failed.
10271027

1028+
Meaning=Meaning
1029+
Symbol=Symbol
1030+
10281031
Entry\ type=Entry type
10291032
Export\ as\ csv\ file=Export as csv file
10301033
Export\ as\ txt\ file=Export as txt file

‎jablib/src/test/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriterTest.java‎

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,12 @@ void checkSimpleLibrary(@TempDir Path tempDir) throws IOException {
5050
| Article | first | o | - |
5151
| Article | second | - | ? |
5252
53-
x | required field is present
54-
o | optional field is present
55-
? | unknown field is present
56-
- | field is absent
53+
| Symbol | Meaning |
54+
| ------ | ------------------------- |
55+
| x | required field is present |
56+
| o | optional field is present |
57+
| ? | unknown field is present |
58+
| - | field is absent |
5759
""", Files.readString(txtFile).replace("\r\n", "\n"));
5860
}
5961

@@ -81,10 +83,45 @@ void checkDifferentOutputSymbols(@TempDir Path tempDir) throws IOException {
8183
| ---------- | ------------ | ------ | ----- | ----- |
8284
| Article | first | ? | o | x |
8385
84-
x | required field is present
85-
o | optional field is present
86-
? | unknown field is present
87-
- | field is absent
86+
| Symbol | Meaning |
87+
| ------ | ------------------------- |
88+
| x | required field is present |
89+
| o | optional field is present |
90+
| ? | unknown field is present |
91+
| - | field is absent |
92+
""", Files.readString(txtFile).replace("\r\n", "\n"));
93+
}
94+
95+
@Test
96+
void checkVeryLongCitationKey(@TempDir Path tempDir) throws IOException {
97+
UnknownField customField = new UnknownField("custom");
98+
BibEntry first = new BibEntry(StandardEntryType.Article, "first-very-long-key")
99+
.withField(StandardField.AUTHOR, "Author One") // required
100+
.withField(StandardField.TITLE, "Title") // required
101+
.withField(StandardField.PAGES, "some pages") // optional
102+
.withField(customField, "custom"); // unknown
103+
BibEntry second = new BibEntry(StandardEntryType.Article, "second")
104+
.withField(StandardField.AUTHOR, "Author One");
105+
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(List.of(first, second));
106+
107+
Path txtFile = tempDir.resolve("checkDifferentOutputSymbols-result.txt");
108+
try (Writer writer = new OutputStreamWriter(Files.newOutputStream(txtFile));
109+
BibliographyConsistencyCheckResultTxtWriter BibliographyConsistencyCheckResultTxtWriter = new BibliographyConsistencyCheckResultTxtWriter(result, writer, false)) {
110+
BibliographyConsistencyCheckResultTxtWriter.writeFindings();
111+
}
112+
assertEquals("""
113+
Field Presence Consistency Check Result
114+
115+
| entry type | citation key | Custom | Pages | Title |
116+
| ---------- | ------------------- | ------ | ----- | ----- |
117+
| Article | first-very-long-key | ? | o | x |
118+
119+
| Symbol | Meaning |
120+
| ------ | ------------------------- |
121+
| x | required field is present |
122+
| o | optional field is present |
123+
| ? | unknown field is present |
124+
| - | field is absent |
88125
""", Files.readString(txtFile).replace("\r\n", "\n"));
89126
}
90127

@@ -106,11 +143,16 @@ void checkComplexLibrary(@TempDir Path tempDir) throws IOException {
106143
.withField(StandardField.AUTHOR, "Author One")
107144
.withField(StandardField.YEAR, "2024")
108145
.withField(StandardField.PUBLISHER, "publisher");
146+
// Entry added to check for alphabetical ordering of citation keys
109147
BibEntry fifth = new BibEntry(StandardEntryType.InProceedings, "fifth")
148+
.withField(StandardField.AUTHOR, "Author One")
149+
.withField(StandardField.LOCATION, "location")
150+
.withField(StandardField.YEAR, "2024");
151+
BibEntry sixth = new BibEntry(StandardEntryType.InProceedings, "sixth")
110152
.withField(StandardField.AUTHOR, "Author One")
111153
.withField(StandardField.YEAR, "2024");
112154

113-
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(List.of(first, second, third, fourth, fifth));
155+
BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(List.of(first, second, third, fourth, fifth, sixth));
114156

115157
Path txtFile = tempDir.resolve("checkSimpleLibrary-result.txt");
116158
try (Writer writer = new OutputStreamWriter(Files.newOutputStream(txtFile));
@@ -120,17 +162,20 @@ void checkComplexLibrary(@TempDir Path tempDir) throws IOException {
120162
assertEquals("""
121163
Field Presence Consistency Check Result
122164
123-
| entry type | citation key | Location | Pages | Publisher |
124-
| ------------- | ------------- | -------- | ----- | --------- |
125-
| Article | first | - | o | - |
126-
| Article | second | - | - | ? |
127-
| InProceedings | fourth | - | - | o |
128-
| InProceedings | third | ? | o | - |
129-
130-
x | required field is present
131-
o | optional field is present
132-
? | unknown field is present
133-
- | field is absent
165+
| entry type | citation key | Location | Pages | Publisher |
166+
| ------------- | ------------ | -------- | ----- | --------- |
167+
| Article | first | - | o | - |
168+
| Article | second | - | - | ? |
169+
| InProceedings | fifth | ? | - | - |
170+
| InProceedings | fourth | - | - | o |
171+
| InProceedings | third | ? | o | - |
172+
173+
| Symbol | Meaning |
174+
| ------ | ------------------------- |
175+
| x | required field is present |
176+
| o | optional field is present |
177+
| ? | unknown field is present |
178+
| - | field is absent |
134179
""", Files.readString(txtFile).replace("\r\n", "\n"));
135180
}
136181

0 commit comments

Comments
 (0)