diff --git a/jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByCitationKeyComparator.java b/jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByCitationKeyComparator.java new file mode 100644 index 00000000000..e9cdcd8b781 --- /dev/null +++ b/jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByCitationKeyComparator.java @@ -0,0 +1,29 @@ +package org.jabref.logic.bibtex.comparator; + +import java.util.Comparator; + +import org.jabref.model.entry.BibEntry; + +public class BibEntryByCitationKeyComparator implements Comparator { + @Override + public int compare(BibEntry e1, BibEntry e2) { + boolean e1HasCitationKey = e1.hasCitationKey(); + boolean e2HasCitationKey = e2.hasCitationKey(); + + if (!e1HasCitationKey && !e2HasCitationKey) { + return 0; + } + + if (e1HasCitationKey && !e2HasCitationKey) { + return -1; + } + + if (!e1HasCitationKey && e2HasCitationKey) { + return 1; + } + + assert e1HasCitationKey && e2HasCitationKey; + + return e1.getCitationKey().get().compareTo(e2.getCitationKey().get()); + } +} diff --git a/jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByFieldsComparator.java b/jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByFieldsComparator.java new file mode 100644 index 00000000000..e1778a9bf98 --- /dev/null +++ b/jablib/src/main/java/org/jabref/logic/bibtex/comparator/BibEntryByFieldsComparator.java @@ -0,0 +1,30 @@ +package org.jabref.logic.bibtex.comparator; + +import java.util.Comparator; +import java.util.Iterator; + +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.Field; + +/** + * Sorts entries by the number of fields and then by the field names. + */ +public class BibEntryByFieldsComparator implements Comparator { + @Override + public int compare(BibEntry e1, BibEntry e2) { + int sizeComparison = e1.getFields().size() - e2.getFields().size(); + if (sizeComparison != 0) { + return sizeComparison; + } + Iterator it1 = e1.getFields().stream().map(Field::getName).sorted().iterator(); + Iterator it2 = e2.getFields().stream().map(Field::getName).sorted().iterator(); + while (it1.hasNext() && it2.hasNext()) { + int fieldComparison = it1.next().compareTo(it2.next()); + if (fieldComparison != 0) { + return fieldComparison; + } + } + assert !it1.hasNext() && !it2.hasNext(); + return 0; + } +} diff --git a/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java b/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java index 9b51ebb1f52..15fb9111d6e 100644 --- a/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java +++ b/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheck.java @@ -4,12 +4,14 @@ import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.SequencedCollection; import java.util.Set; +import org.jabref.logic.bibtex.comparator.BibEntryByCitationKeyComparator; +import org.jabref.logic.bibtex.comparator.BibEntryByFieldsComparator; +import org.jabref.logic.bibtex.comparator.FieldComparatorStack; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.Field; import org.jabref.model.entry.types.EntryType; @@ -55,38 +57,23 @@ public Result check(List entries) { return; } - List sortedEntries = entryTypeToEntriesMap + List> comparators = List.of( + new BibEntryByCitationKeyComparator(), + new BibEntryByFieldsComparator()); + FieldComparatorStack comparatorStack = new FieldComparatorStack<>(comparators); + + List differingEntries = entryTypeToEntriesMap .get(entryType).stream() .filter(entry -> !entry.getFields().equals(commonFields)) - .sorted(getBibEntryComparator()).toList(); - resultMap.put(entryType, new EntryTypeResult(uniqueFields, sortedEntries)); + .sorted(comparatorStack) + .toList(); + + resultMap.put(entryType, new EntryTypeResult(uniqueFields, differingEntries)); }); return new Result(resultMap); } - /** - * Sorts entries by the number of fields and then by the field names. - */ - private static Comparator getBibEntryComparator() { - return (e1, e2) -> { - int sizeComparison = e1.getFields().size() - e2.getFields().size(); - if (sizeComparison != 0) { - return sizeComparison; - } - Iterator it1 = e1.getFields().stream().map(Field::getName).sorted().iterator(); - Iterator it2 = e2.getFields().stream().map(Field::getName).sorted().iterator(); - while (it1.hasNext() && it2.hasNext()) { - int fieldComparison = it1.next().compareTo(it2.next()); - if (fieldComparison != 0) { - return fieldComparison; - } - } - assert !it1.hasNext() && !it2.hasNext(); - return 0; - }; - } - private static void collectEntriesIntoMaps(List entries, Map> entryTypeToFieldsInAnyEntryMap, Map> entryTypeToFieldsInAllEntriesMap, Map> entryTypeToEntriesMap) { entries.forEach(entry -> { EntryType entryType = entry.getType(); diff --git a/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriter.java b/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriter.java index 33927601b5a..5a5de7b15ca 100644 --- a/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriter.java +++ b/jablib/src/main/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriter.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.io.Writer; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Set; import java.util.StringJoiner; @@ -53,46 +54,43 @@ public void writeFindings() throws IOException { super.writeFindings(); if (!isPorcelain) { + int widthSymbol = Localization.lang("Symbol").length(); + int widthMeaning = Collections.max(List.of( + Localization.lang("Meaning").length(), + Localization.lang("required field is present").length(), + Localization.lang("optional field is present").length(), + Localization.lang("unknown field is present").length(), + Localization.lang("field is absent").length() + )); + writer.write("\n"); - writer.write("%s | %s\n".formatted(REQUIRED_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("required field is present"))); - writer.write("%s | %s\n".formatted(OPTIONAL_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("optional field is present"))); - writer.write("%s | %s\n".formatted(UNKNOWN_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("unknown field is present"))); - writer.write("%s | %s\n".formatted(UNSET_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("field is absent"))); + writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(Localization.lang("Symbol"), Localization.lang("Meaning"))); + writer.write(("| " + "-".repeat(widthSymbol) + " | " + "-".repeat(widthMeaning) + " |\n").formatted("--", "--")); + writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(REQUIRED_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("required field is present"))); + writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(OPTIONAL_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("optional field is present"))); + writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(UNKNOWN_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("unknown field is present"))); + writer.write(("| %-" + widthSymbol + "s | %-" + widthMeaning + "s |\n").formatted(UNSET_FIELD_AT_ENTRY_TYPE_CELL_ENTRY, Localization.lang("field is absent"))); } } private void initializeColumnWidths() { columnWidths = new ArrayList<>(columnNames.size()); - Integer max = getColumnWidthOfEntryTypes(); - columnWidths.add(max); + int entryTypeWidth = "entry type".length(); + int citationKeyWidth = "citation key".length(); - max = getColumnWidthOfCitationKeys(max); - columnWidths.add(max); + for (var keysAndValue : result.entryTypeToResultMap().entrySet()) { + entryTypeWidth = Math.max(entryTypeWidth, keysAndValue.getKey().getDisplayName().length()); + for (var entry : keysAndValue.getValue().sortedEntries()) { + citationKeyWidth = Math.max(citationKeyWidth, entry.getCitationKey().orElse("").length()); + } + } + columnWidths.add(entryTypeWidth); + columnWidths.add(citationKeyWidth); columnWidths.addAll(columnNames.stream().skip(2).map(String::length).toList()); } - private Integer getColumnWidthOfEntryTypes() { - int max = result.entryTypeToResultMap().keySet() - .stream() - .map(entryType -> entryType.getDisplayName().length()) - .max(Integer::compareTo) - .get(); - max = Math.max(max, "entry type".length()); - return max; - } - - private Integer getColumnWidthOfCitationKeys(Integer max) { - result.entryTypeToResultMap().values() - .stream() - .flatMap(entryTypeResult -> entryTypeResult.sortedEntries().stream()) - .map(entry -> entry.getCitationKey().orElse("").length()) - .max(Integer::compareTo) - .get(); - return Math.max(max, "citation key".length()); - } - @Override protected void writeBibEntry(BibEntry bibEntry, String entryType, Set requiredFields, Set optionalFields) throws IOException { List theRecord = getFindingsAsList(bibEntry, entryType, requiredFields, optionalFields); diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties index 7ac221f6db1..f25eb4ecdb7 100644 --- a/jablib/src/main/resources/l10n/JabRef_en.properties +++ b/jablib/src/main/resources/l10n/JabRef_en.properties @@ -1025,6 +1025,9 @@ Consistency\ check\ completed=Consistency check completed Check\ consistency=Check consistency Consistency\ check\ failed.=Consistency check failed. +Meaning=Meaning +Symbol=Symbol + Entry\ type=Entry type Export\ as\ csv\ file=Export as csv file Export\ as\ txt\ file=Export as txt file diff --git a/jablib/src/test/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriterTest.java b/jablib/src/test/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriterTest.java index 596ba6cc47e..dd6c005b332 100644 --- a/jablib/src/test/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriterTest.java +++ b/jablib/src/test/java/org/jabref/logic/quality/consistency/BibliographyConsistencyCheckResultTxtWriterTest.java @@ -50,10 +50,12 @@ void checkSimpleLibrary(@TempDir Path tempDir) throws IOException { | Article | first | o | - | | Article | second | - | ? | - x | required field is present - o | optional field is present - ? | unknown field is present - - | field is absent + | Symbol | Meaning | + | ------ | ------------------------- | + | x | required field is present | + | o | optional field is present | + | ? | unknown field is present | + | - | field is absent | """, Files.readString(txtFile).replace("\r\n", "\n")); } @@ -81,10 +83,45 @@ void checkDifferentOutputSymbols(@TempDir Path tempDir) throws IOException { | ---------- | ------------ | ------ | ----- | ----- | | Article | first | ? | o | x | - x | required field is present - o | optional field is present - ? | unknown field is present - - | field is absent + | Symbol | Meaning | + | ------ | ------------------------- | + | x | required field is present | + | o | optional field is present | + | ? | unknown field is present | + | - | field is absent | + """, Files.readString(txtFile).replace("\r\n", "\n")); + } + + @Test + void checkVeryLongCitationKey(@TempDir Path tempDir) throws IOException { + UnknownField customField = new UnknownField("custom"); + BibEntry first = new BibEntry(StandardEntryType.Article, "first-very-long-key") + .withField(StandardField.AUTHOR, "Author One") // required + .withField(StandardField.TITLE, "Title") // required + .withField(StandardField.PAGES, "some pages") // optional + .withField(customField, "custom"); // unknown + BibEntry second = new BibEntry(StandardEntryType.Article, "second") + .withField(StandardField.AUTHOR, "Author One"); + BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(List.of(first, second)); + + Path txtFile = tempDir.resolve("checkDifferentOutputSymbols-result.txt"); + try (Writer writer = new OutputStreamWriter(Files.newOutputStream(txtFile)); + BibliographyConsistencyCheckResultTxtWriter BibliographyConsistencyCheckResultTxtWriter = new BibliographyConsistencyCheckResultTxtWriter(result, writer, false)) { + BibliographyConsistencyCheckResultTxtWriter.writeFindings(); + } + assertEquals(""" + Field Presence Consistency Check Result + + | entry type | citation key | Custom | Pages | Title | + | ---------- | ------------------- | ------ | ----- | ----- | + | Article | first-very-long-key | ? | o | x | + + | Symbol | Meaning | + | ------ | ------------------------- | + | x | required field is present | + | o | optional field is present | + | ? | unknown field is present | + | - | field is absent | """, Files.readString(txtFile).replace("\r\n", "\n")); } @@ -106,11 +143,16 @@ void checkComplexLibrary(@TempDir Path tempDir) throws IOException { .withField(StandardField.AUTHOR, "Author One") .withField(StandardField.YEAR, "2024") .withField(StandardField.PUBLISHER, "publisher"); + // Entry added to check for alphabetical ordering of citation keys BibEntry fifth = new BibEntry(StandardEntryType.InProceedings, "fifth") + .withField(StandardField.AUTHOR, "Author One") + .withField(StandardField.LOCATION, "location") + .withField(StandardField.YEAR, "2024"); + BibEntry sixth = new BibEntry(StandardEntryType.InProceedings, "sixth") .withField(StandardField.AUTHOR, "Author One") .withField(StandardField.YEAR, "2024"); - BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(List.of(first, second, third, fourth, fifth)); + BibliographyConsistencyCheck.Result result = new BibliographyConsistencyCheck().check(List.of(first, second, third, fourth, fifth, sixth)); Path txtFile = tempDir.resolve("checkSimpleLibrary-result.txt"); try (Writer writer = new OutputStreamWriter(Files.newOutputStream(txtFile)); @@ -120,17 +162,20 @@ void checkComplexLibrary(@TempDir Path tempDir) throws IOException { assertEquals(""" Field Presence Consistency Check Result - | entry type | citation key | Location | Pages | Publisher | - | ------------- | ------------- | -------- | ----- | --------- | - | Article | first | - | o | - | - | Article | second | - | - | ? | - | InProceedings | fourth | - | - | o | - | InProceedings | third | ? | o | - | - - x | required field is present - o | optional field is present - ? | unknown field is present - - | field is absent + | entry type | citation key | Location | Pages | Publisher | + | ------------- | ------------ | -------- | ----- | --------- | + | Article | first | - | o | - | + | Article | second | - | - | ? | + | InProceedings | fifth | ? | - | - | + | InProceedings | fourth | - | - | o | + | InProceedings | third | ? | o | - | + + | Symbol | Meaning | + | ------ | ------------------------- | + | x | required field is present | + | o | optional field is present | + | ? | unknown field is present | + | - | field is absent | """, Files.readString(txtFile).replace("\r\n", "\n")); }