Skip to content
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

Preserve casing of field names for customize entry types #9993

Merged
merged 13 commits into from
Jun 12, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We improved the Medline importer to correctly import ISO dates for `revised`. [#9536](https://github.com/JabRef/jabref/issues/9536)
- To avoid cluttering of the directory, We always delete the `.sav` file upon successful write. [#9675](https://github.com/JabRef/jabref/pull/9675)
- We improved the unlinking/deletion of multiple linked files of an entry using the <kbd>Delete</kbd> key. [#9473](https://github.com/JabRef/jabref/issues/9473)
- The field names of customized entry types are now exchanged preserving the case. [#9993](https://github.com/JabRef/jabref/pull/9993)
- We moved the custom entry types dialog into the preferences dialog. [#9760](https://github.com/JabRef/jabref/pull/9760)
- We moved the manage content selectors dialog to the library properties. [#9768](https://github.com/JabRef/jabref/pull/9768)
- We moved the preferences menu command from the options menu to the file menu. [#9768](https://github.com/JabRef/jabref/pull/9768)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@
import org.jabref.model.entry.types.EntryTypeFactory;
import org.jabref.preferences.PreferencesService;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class ImportCustomEntryTypesDialogViewModel {

private static final Logger LOGGER = LoggerFactory.getLogger(ImportCustomEntryTypesDialogViewModel.class);

private final BibDatabaseMode mode;
private final PreferencesService preferencesService;

Expand All @@ -30,6 +35,8 @@ public ImportCustomEntryTypesDialogViewModel(BibDatabaseMode mode, List<BibEntry
newTypes.add(customType);
} else {
if (!EntryTypeFactory.isEqualNameAndFieldBased(customType, currentlyStoredType.get())) {
LOGGER.info("currently stored type: {}", currentlyStoredType.get());
LOGGER.info("type provided by library: {}", customType);
differentCustomizationTypes.add(customType);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ public void initialize() {
dialogService.showErrorDialogAndWait(Localization.lang(
"A string with the label '%0' already exists.",
cellEvent.getNewValue()));

cellItem.setLabel(cellEvent.getOldValue());
} else {
cellItem.setLabel(cellEvent.getNewValue());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.jabref.gui.preferences.customentrytypes;

import java.util.Locale;

import javafx.application.Platform;
import javafx.beans.property.ReadOnlyStringWrapper;
import javafx.collections.ObservableList;
Expand Down Expand Up @@ -35,8 +37,6 @@
import org.jabref.model.database.BibDatabaseMode;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.UnknownField;
import org.jabref.model.strings.StringUtil;

import com.airhacks.afterburner.views.ViewLoader;
import com.tobiasdiez.easybind.EasyBind;
Expand Down Expand Up @@ -146,30 +146,32 @@ private void setupEntryTypesTable() {
}

private void setupFieldsTable() {
fieldNameColumn.setCellValueFactory(item -> item.getValue().nameProperty());
fieldNameColumn.setCellValueFactory(item -> item.getValue().displayNameProperty());
fieldNameColumn.setCellFactory(TextFieldTableCell.forTableColumn());
fieldNameColumn.setEditable(true);
fieldNameColumn.setOnEditCommit(
(TableColumn.CellEditEvent<FieldViewModel, String> event) -> {
// This makes the displayed name consistent to org.jabref.model.entry.field.Field #getDisplayName()
String newFieldValue = StringUtil.capitalizeFirst(event.getNewValue());
UnknownField field = (UnknownField) event.getRowValue().getField();
EntryTypeViewModel selectedEntryType = viewModel.selectedEntryTypeProperty().get();
ObservableList<FieldViewModel> entryFields = selectedEntryType.fields();
// The first predicate will check if the user input the original field name or doesn't edit anything after double click
boolean fieldExists = !newFieldValue.equals(field.getDisplayName()) && entryFields.stream().anyMatch(fieldViewModel ->
fieldViewModel.nameProperty().getValue().equalsIgnoreCase(newFieldValue));

if (fieldExists) {
dialogService.notify(Localization.lang("Unable to change field name. \"%0\" already in use.", newFieldValue));
event.getTableView().edit(-1, null);
event.getTableView().refresh();
} else {
event.getRowValue().setField(newFieldValue);
field.setName(newFieldValue);
event.getTableView().refresh();
}
});
fieldNameColumn.setOnEditCommit((TableColumn.CellEditEvent<FieldViewModel, String> event) -> {
String newDisplayName = event.getNewValue();
if (newDisplayName.isBlank()) {
dialogService.notify(Localization.lang("Name cannot be empty"));
event.getTableView().edit(-1, null);
event.getTableView().refresh();
return;
}

FieldViewModel fieldViewModel = event.getRowValue();
String currentDisplayName = fieldViewModel.displayNameProperty().getValue();
EntryTypeViewModel selectedEntryType = viewModel.selectedEntryTypeProperty().get();
ObservableList<FieldViewModel> entryFields = selectedEntryType.fields();
// The first predicate will check if the user input the original field name or doesn't edit anything after double click
boolean fieldExists = !newDisplayName.equals(currentDisplayName) && viewModel.displayNameExists(newDisplayName);
if (fieldExists) {
dialogService.notify(Localization.lang("Unable to change field name. \"%0\" already in use.", newDisplayName));
event.getTableView().edit(-1, null);
} else {
fieldViewModel.displayNameProperty().setValue(newDisplayName);
}
event.getTableView().refresh();
});

fieldTypeColumn.setCellFactory(CheckBoxTableCell.forTableColumn(fieldTypeColumn));
fieldTypeColumn.setCellValueFactory(item -> item.getValue().requiredProperty());
Expand All @@ -182,7 +184,7 @@ private void setupFieldsTable() {
fieldTypeActionColumn.setSortable(false);
fieldTypeActionColumn.setReorderable(false);
fieldTypeActionColumn.setEditable(false);
fieldTypeActionColumn.setCellValueFactory(cellData -> cellData.getValue().nameProperty());
fieldTypeActionColumn.setCellValueFactory(cellData -> cellData.getValue().displayNameProperty());

new ValueTableCellFactory<FieldViewModel, String>()
.withGraphic(item -> IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public CustomEntryTypesTabViewModel(BibDatabaseMode mode,
}

public void setValues() {
if (this.entryTypesWithFields.size() > 0) {
if (!this.entryTypesWithFields.isEmpty()) {
this.entryTypesWithFields.clear();
}
Collection<BibEntryType> allTypes = entryTypesManager.getAllTypes(bibDatabaseMode);
Expand All @@ -105,12 +105,12 @@ public void storeSettings() {

multilineFields.addAll(allFields.stream()
.filter(FieldViewModel::isMultiline)
.map(FieldViewModel::getField)
.map(FieldViewModel::toField)
.toList());

List<OrFields> required = allFields.stream()
.filter(FieldViewModel::isRequired)
.map(FieldViewModel::getField)
.map(FieldViewModel::toField)
.map(OrFields::new)
.collect(Collectors.toList());
List<BibField> fields = allFields.stream().map(FieldViewModel::toBibField).collect(Collectors.toList());
Expand Down Expand Up @@ -144,24 +144,28 @@ public void removeEntryType(EntryTypeViewModel focusedItem) {

public void addNewField() {
Field field = newFieldToAdd.getValue();
ObservableList<FieldViewModel> entryFields = this.selectedEntryType.getValue().fields();
boolean fieldExists = entryFields.stream().anyMatch(fieldViewModel ->
fieldViewModel.nameProperty().getValue().equals(field.getDisplayName()));
boolean fieldExists = displayNameExists(field.getDisplayName());

if (!fieldExists) {
if (fieldExists) {
dialogService.showWarningDialogAndWait(
Localization.lang("Duplicate fields"),
Localization.lang("Warning: You added field \"%0\" twice. Only one will be kept.", field.getDisplayName()));
} else {
this.selectedEntryType.getValue().addField(new FieldViewModel(
field,
FieldViewModel.Mandatory.REQUIRED,
FieldPriority.IMPORTANT,
false));
} else {
dialogService.showWarningDialogAndWait(
Localization.lang("Duplicate fields"),
Localization.lang("Warning: You added field \"%0\" twice. Only one will be kept.", field.getDisplayName()));
}
newFieldToAddProperty().setValue(null);
}

public boolean displayNameExists(String displayName) {
ObservableList<FieldViewModel> entryFields = this.selectedEntryType.getValue().fields();
return entryFields.stream().anyMatch(fieldViewModel ->
fieldViewModel.displayNameProperty().getValue().equals(displayName));
}

public void removeField(FieldViewModel focusedItem) {
selectedEntryType.getValue().removeField(focusedItem);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ public class EntryTypeViewModel {

public EntryTypeViewModel(BibEntryType entryType, Predicate<Field> isMultiline) {
this.entryType.set(entryType);

List<FieldViewModel> allFieldsForType = entryType.getAllBibFields()
.stream().map(bibField -> new FieldViewModel(bibField.field(),
entryType.isRequired(bibField.field()) ? Mandatory.REQUIRED : Mandatory.OPTIONAL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@
import org.jabref.logic.l10n.Localization;
import org.jabref.model.entry.field.BibField;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.FieldPriority;
import org.jabref.model.entry.field.FieldProperty;
import org.jabref.model.entry.field.UnknownField;

import com.tobiasdiez.easybind.EasyBind;

public class FieldViewModel {

private final Field field;
private final StringProperty fieldName = new SimpleStringProperty("");
private final StringProperty displayName = new SimpleStringProperty("");
private final BooleanProperty required = new SimpleBooleanProperty();
private final BooleanProperty multiline = new SimpleBooleanProperty();
private final ObjectProperty<FieldPriority> priorityProperty = new SimpleObjectProperty<>();
Expand All @@ -27,27 +28,14 @@ public FieldViewModel(Field field,
Mandatory required,
FieldPriority priorityProperty,
boolean multiline) {
this.field = field;
this.fieldName.setValue(field.getDisplayName());
this.displayName.setValue(field.getDisplayName());
this.required.setValue(required == Mandatory.REQUIRED);
this.priorityProperty.setValue(priorityProperty);
this.multiline.setValue(multiline);

EasyBind.subscribe(this.multiline, multi -> {
if (multi) {
this.field.getProperties().add(FieldProperty.MULTILINE_TEXT);
} else {
this.field.getProperties().remove(FieldProperty.MULTILINE_TEXT);
}
});
}

public Field getField() {
return field;
}

public StringProperty nameProperty() {
return fieldName;
public StringProperty displayNameProperty() {
return displayName;
}

public BooleanProperty requiredProperty() {
Expand All @@ -70,21 +58,26 @@ public FieldPriority getPriority() {
return priorityProperty.getValue();
}

public void setField(String field) {
this.fieldName.setValue(field);
public Field toField() {
// If the field name is known by JabRef, JabRef's casing will win.
// If the field is not known by JabRef (UnknownField), the new casing will be taken.
Field field = FieldFactory.parseField(displayName.getValue());
if (multiline.getValue()) {
field.getProperties().add(FieldProperty.MULTILINE_TEXT);
}
return field;
}

public BibField toBibField() {
return new BibField(field, priorityProperty.getValue());
return new BibField(toField(), priorityProperty.getValue());
}

@Override
public String toString() {
return field.getDisplayName();
return displayName.getValue();
}

public enum Mandatory {

REQUIRED(Localization.lang("Required")),
OPTIONAL(Localization.lang("Optional"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ public static String serializeCustomEntryTypes(BibEntryType entryType) {
builder.append(MetaData.ENTRYTYPE_FLAG);
builder.append(entryType.getType().getName());
builder.append(": req[");
builder.append(FieldFactory.serializeOrFieldsList(entryType.getRequiredFields()));
builder.append(FieldFactory.serializeOrFieldsListUsingDisplayName(entryType.getRequiredFields()));
builder.append("] opt[");
builder.append(FieldFactory.serializeFieldsList(
builder.append(FieldFactory.serializeFieldsListUsingDisplayName(
entryType.getOptionalFields()
.stream()
.map(BibField::field)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,12 @@ public static Optional<BibEntryType> parseCustomEntryType(String comment) {
EntryType type = EntryTypeFactory.parse(rest.substring(0, indexEndOfName));
String reqFields = fieldsDescription.substring(4, indexEndOfRequiredFields);
String optFields = fieldsDescription.substring(indexEndOfRequiredFields + 6, indexEndOfOptionalFields);

BibEntryTypeBuilder entryTypeBuilder = new BibEntryTypeBuilder()
.withType(type)
.withImportantFields(FieldFactory.parseFieldList(optFields))
.withRequiredFields(FieldFactory.parseOrFieldsList(reqFields));
.withRequiredFields(FieldFactory.parseOrFieldsList(reqFields))
// Important fields are optional fields, but displayed first. Thus, they do not need to be separated by "/".
// See org.jabref.model.entry.field.FieldPriority for details on important optional fields.
.withImportantFields(FieldFactory.parseFieldList(optFields));
return Optional.of(entryTypeBuilder.build());
}

Expand Down
16 changes: 10 additions & 6 deletions src/main/java/org/jabref/model/entry/BibEntryType.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.Set;
import java.util.stream.Collectors;

import org.jabref.logic.util.OS;
import org.jabref.model.database.BibDatabaseMode;
import org.jabref.model.entry.field.BibField;
import org.jabref.model.entry.field.Field;
Expand All @@ -24,14 +25,16 @@ public class BibEntryType implements Comparable<BibEntryType> {
/**
* Provides an enriched EntryType with information about defined standards as mandatory fields etc.
*
* A builder is available at {@link BibEntryTypeBuilder}
*
* @param type The EntryType this BibEntryType is wrapped around.
* @param fields A BibFields list of all fields, including the required fields
* @param requiredFields A OrFields list of just the required fields
*/
public BibEntryType(EntryType type, Collection<BibField> fields, Collection<OrFields> requiredFields) {
this.type = Objects.requireNonNull(type);
this.requiredFields = new LinkedHashSet<>(requiredFields);
this.fields = new LinkedHashSet<>(fields);
this.requiredFields = new LinkedHashSet<>(requiredFields);
}

public EntryType getType() {
Expand Down Expand Up @@ -146,13 +149,14 @@ public int hashCode() {
return Objects.hash(type, requiredFields, fields);
}

/**
* This representation is also used in the UI, thus it is formatted more pretty
*/
@Override
public String toString() {
return "BibEntryType{" +
"type=" + type +
", requiredFields=" + requiredFields +
", fields=" + fields +
'}';
return "type = " + type + "," + OS.NEWLINE +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this will break many tests! Rather add a new method

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduced org.jabref.gui.importer.BibEntryTypePrefsAndFileViewModel to really fix that comment.

image

"allFields = " + fields + "," + OS.NEWLINE +
"requiredFields = " + requiredFields;
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/model/entry/field/BibField.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public int hashCode() {
@Override
public String toString() {
return "BibField{" +
"field=" + field.getName() +
"field=" + field.getDisplayName() +
", priority=" + priority +
'}';
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/jabref/model/entry/field/Field.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ public interface Field {
/**
* properties contains mappings to tell the EntryEditor to add a specific function to this field,
* for instance a dropdown for selecting the month for the month field.
*
* Note that this set needs to be mutable. This is required, because we allow standard fields to be modifiable via the UI.
*/
Set<FieldProperty> getProperties();

Expand Down
Loading