Skip to content

Commit

Permalink
Show a warning in the merge dialog when authors are the same but form…
Browse files Browse the repository at this point in the history
…atted 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>
  • Loading branch information
HoussemNasri and ThiloteE authored Sep 2, 2022
1 parent 5a34184 commit 7239503
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -35,15 +37,17 @@
*/
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;
private final MergedFieldCell mergedValueCell;

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) {
Expand Down Expand Up @@ -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();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
3 changes: 3 additions & 0 deletions src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7239503

Please sign in to comment.