From 72395033d47c4e83f0e6523057ef93e2bd93a2ed Mon Sep 17 00:00:00 2001 From: Houssem Nasri Date: Fri, 2 Sep 2022 16:58:24 +0100 Subject: [PATCH] Show a warning in the merge dialog when authors are the same but formatted differently (#9088) * Show a warning when the authors are the same but formatted differently * Update CHANGELOG.md * Link issue rather than pr in the changelog * Checkstyle * Show the warning for all fields with person names * Modify warning message Co-authored-by: ThiloteE <73715071+thilotee@users.noreply.github.com> * Use an info icon rather than a warning * Checkstyle * Add a comment for why InfoButton command is empty * Delete FieldRowController.java * Hide diffs if persons names are the same * i18n Co-authored-by: ThiloteE <73715071+thilotee@users.noreply.github.com> --- CHANGELOG.md | 1 + .../newmergedialog/FieldRowView.java | 18 ++++--- .../PersonsNameFieldRowView.java | 34 +++++++++++++ .../newmergedialog/ThreeWayMergeView.java | 8 ++- .../cell/sidebuttons/InfoButton.java | 51 +++++++++++++++++++ src/main/resources/l10n/JabRef_en.properties | 3 ++ 6 files changed, 108 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/jabref/gui/mergeentries/newmergedialog/PersonsNameFieldRowView.java create mode 100644 src/main/java/org/jabref/gui/mergeentries/newmergedialog/cell/sidebuttons/InfoButton.java diff --git a/CHANGELOG.md b/CHANGELOG.md index b1bdcb91d05..23f76110599 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - On startup, JabRef notifies the user if there were parsing errors during opening. - We integrated a new three-way merge UI for merging entries in the Entries Merger Dialog, the Duplicate Resolver Dialog, the Entry Importer Dialog, and the External Changes Resolver Dialog. [#8945](https://github.com/JabRef/jabref/pull/8945) - We added the ability to merge groups, keywords, comments and files when merging entries. [#9022](https://github.com/JabRef/jabref/pull/9022) +- We added a warning message next to the authors field in the merge dialog to warn users when the authors are the same but formatted differently. [#8745](https://github.com/JabRef/jabref/issues/8745) ### Changed diff --git a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/FieldRowView.java b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/FieldRowView.java index 43561b9b5c5..9b50e95817d 100644 --- a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/FieldRowView.java +++ b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/FieldRowView.java @@ -5,7 +5,9 @@ import javax.swing.undo.CannotUndoException; import javax.swing.undo.CompoundEdit; +import javafx.beans.property.BooleanProperty; import javafx.beans.property.ReadOnlyStringProperty; +import javafx.beans.property.SimpleBooleanProperty; import javafx.scene.control.ToggleGroup; import javafx.scene.layout.GridPane; @@ -35,6 +37,10 @@ */ public class FieldRowView { private static final Logger LOGGER = LoggerFactory.getLogger(FieldRowView.class); + + protected final FieldRowViewModel viewModel; + + protected final BooleanProperty shouldShowDiffs = new SimpleBooleanProperty(true); private final FieldNameCell fieldNameCell; private final FieldValueCell leftValueCell; private final FieldValueCell rightValueCell; @@ -42,8 +48,6 @@ public class FieldRowView { private final ToggleGroup toggleGroup = new ToggleGroup(); - private final FieldRowViewModel viewModel; - private final CompoundEdit fieldsMergedEdit = new CompoundEdit(); public FieldRowView(Field field, BibEntry leftEntry, BibEntry rightEntry, BibEntry mergedEntry, FieldMergerFactory fieldMergerFactory, int rowIndex) { @@ -160,10 +164,12 @@ public void showDiff(ShowDiffConfig diffConfig) { StyleClassedTextArea rightLabel = rightValueCell.getStyleClassedLabel(); // Clearing old diff styles based on previous diffConfig hideDiff(); - if (diffConfig.diffView() == ThreeWayMergeToolbar.DiffView.UNIFIED) { - new UnifiedDiffHighlighter(leftLabel, rightLabel, diffConfig.diffHighlightingMethod()).highlight(); - } else { - new SplitDiffHighlighter(leftLabel, rightLabel, diffConfig.diffHighlightingMethod()).highlight(); + if (shouldShowDiffs.get()) { + if (diffConfig.diffView() == ThreeWayMergeToolbar.DiffView.UNIFIED) { + new UnifiedDiffHighlighter(leftLabel, rightLabel, diffConfig.diffHighlightingMethod()).highlight(); + } else { + new SplitDiffHighlighter(leftLabel, rightLabel, diffConfig.diffHighlightingMethod()).highlight(); + } } } diff --git a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/PersonsNameFieldRowView.java b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/PersonsNameFieldRowView.java new file mode 100644 index 00000000000..e42bb91743b --- /dev/null +++ b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/PersonsNameFieldRowView.java @@ -0,0 +1,34 @@ +package org.jabref.gui.mergeentries.newmergedialog; + +import org.jabref.gui.mergeentries.newmergedialog.cell.sidebuttons.InfoButton; +import org.jabref.gui.mergeentries.newmergedialog.fieldsmerger.FieldMergerFactory; +import org.jabref.logic.importer.AuthorListParser; +import org.jabref.logic.l10n.Localization; +import org.jabref.model.entry.AuthorList; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.field.Field; +import org.jabref.model.entry.field.FieldProperty; + +public class PersonsNameFieldRowView extends FieldRowView { + private final AuthorList leftEntryNames; + private final AuthorList rightEntryNames; + + public PersonsNameFieldRowView(Field field, BibEntry leftEntry, BibEntry rightEntry, BibEntry mergedEntry, FieldMergerFactory fieldMergerFactory, int rowIndex) { + super(field, leftEntry, rightEntry, mergedEntry, fieldMergerFactory, rowIndex); + assert field.getProperties().contains(FieldProperty.PERSON_NAMES); + + var authorsParser = new AuthorListParser(); + leftEntryNames = authorsParser.parse(viewModel.getLeftFieldValue()); + rightEntryNames = authorsParser.parse(viewModel.getRightFieldValue()); + + if (!leftEntry.equals(rightEntry) && leftEntryNames.equals(rightEntryNames)) { + showPersonsNamesAreTheSameInfo(); + shouldShowDiffs.set(false); + } + } + + private void showPersonsNamesAreTheSameInfo() { + InfoButton infoButton = new InfoButton(Localization.lang("The %0s are the same. However, the order of field content differs", viewModel.getField().getName())); + getFieldNameCell().addSideButton(infoButton); + } +} diff --git a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeView.java b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeView.java index a61833456f8..06f6861d751 100644 --- a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeView.java +++ b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/ThreeWayMergeView.java @@ -17,6 +17,7 @@ import org.jabref.logic.l10n.Localization; import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.field.Field; +import org.jabref.model.entry.field.FieldProperty; public class ThreeWayMergeView extends VBox { public static final int GRID_COLUMN_MIN_WIDTH = 250; @@ -119,7 +120,12 @@ private Field getFieldAtIndex(int index) { private void addRow(int fieldIndex) { Field field = getFieldAtIndex(fieldIndex); - FieldRowView fieldRow = new FieldRowView(field, getLeftEntry(), getRightEntry(), getMergedEntry(), fieldMergerFactory, fieldIndex); + FieldRowView fieldRow; + if (field.getProperties().contains(FieldProperty.PERSON_NAMES)) { + fieldRow = new PersonsNameFieldRowView(field, getLeftEntry(), getRightEntry(), getMergedEntry(), fieldMergerFactory, fieldIndex); + } else { + fieldRow = new FieldRowView(field, getLeftEntry(), getRightEntry(), getMergedEntry(), fieldMergerFactory, fieldIndex); + } fieldRows.add(fieldIndex, fieldRow); diff --git a/src/main/java/org/jabref/gui/mergeentries/newmergedialog/cell/sidebuttons/InfoButton.java b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/cell/sidebuttons/InfoButton.java new file mode 100644 index 00000000000..da37e2267ce --- /dev/null +++ b/src/main/java/org/jabref/gui/mergeentries/newmergedialog/cell/sidebuttons/InfoButton.java @@ -0,0 +1,51 @@ +package org.jabref.gui.mergeentries.newmergedialog.cell.sidebuttons; + +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; +import javafx.scene.control.Button; + +import org.jabref.gui.Globals; +import org.jabref.gui.actions.Action; +import org.jabref.gui.actions.ActionFactory; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.icon.IconTheme; + +import com.tobiasdiez.easybind.EasyBind; + +public class InfoButton extends Button { + private final StringProperty infoMessage = new SimpleStringProperty(); + private final ActionFactory actionFactory = new ActionFactory(Globals.getKeyPrefs()); + + public InfoButton(String infoMessage) { + setInfoMessage(infoMessage); + configureButton(); + EasyBind.subscribe(infoMessageProperty(), newWarningMessage -> { + configureButton(); + }); + } + + private void configureButton() { + setMaxHeight(Double.MAX_VALUE); + setFocusTraversable(false); + Action mergeAction = new Action.Builder(getInfoMessage()).setIcon(IconTheme.JabRefIcons.INTEGRITY_INFO); + + actionFactory.configureIconButton(mergeAction, new SimpleCommand() { + @Override + public void execute() { + // The info button is not meant to be clickable that's why this is empty + } + }, this); + } + + private void setInfoMessage(String infoMessage) { + infoMessageProperty().set(infoMessage); + } + + public StringProperty infoMessageProperty() { + return infoMessage; + } + + public String getInfoMessage() { + return infoMessage.get(); + } +} diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 141212c61fc..6dff22b68d0 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -2526,3 +2526,6 @@ Merge\ %0=Merge %0 Right\ Entry=Right Entry Unmerge\ %0=Unmerge %0 plain\ text=plain text + +The\ %0s\ are\ the\ same.\ However,\ the\ order\ of\ field\ content\ differs=The %0s are the same. However, the order of field content differs +