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

Refactor BibtexKeyPatternPreferences #6489

Merged
merged 13 commits into from
May 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We merged the functionality "Append contents from a BibTeX library into the currently viewed library" into the "Import into database" functionality. Fixes [#6049](https://github.com/JabRef/jabref/issues/6049).
- We changed the directory where fulltext downloads are stored to the directory set in the import-tab in preferences. [#6381](https://github.com/JabRef/jabref/pull/6381)
- We improved the error message for invalid jstyles. [#6303](https://github.com/JabRef/jabref/issues/6303)
- We changed the section name of 'Advanced' to 'Network' in the preferences and removed some obsolete options.[#6489](https://github.com/JabRef/jabref/pull/6489)
- We improved the context menu of the column "Linked identifiers" of the main table, by truncating their texts, if they are too long. [#6499](https://github.com/JabRef/jabref/issues/6499)

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public BibtexKeyPatternDialog(BasePanel panel) {
this.bibtexKeyPatternPanel = new BibtexKeyPatternPanel(panel);
this.panel = panel;
this.metaData = panel.getBibDatabaseContext().getMetaData();
AbstractBibtexKeyPattern keyPattern = metaData.getCiteKeyPattern(Globals.prefs.getKeyPattern());
AbstractBibtexKeyPattern keyPattern = metaData.getCiteKeyPattern(Globals.prefs.getGlobalBibtexKeyPattern());
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about renaming BibtexKeyPattern to CiteKeyPattern to be consistent with the naming in the metadata class?

Copy link
Member Author

@calixtus calixtus May 21, 2020

Choose a reason for hiding this comment

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

I'm really not sure about this. On one hand it would make sense, because it would abstract this preference beyond bibtex and biblatex. On the other hand, it is closely tangled to the term bibtexKey, which has already become a fixed technical term in JabRef.

I measured the amount of files affected by a refactor of this. About 43 files would change.

bibtexKeyPatternPanel.setValues(keyPattern);
init();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private void buildGUI() {
}

public DatabaseBibtexKeyPattern getKeyPatternAsDatabaseBibtexKeyPattern() {
DatabaseBibtexKeyPattern res = new DatabaseBibtexKeyPattern(Globals.prefs.getKeyPattern());
DatabaseBibtexKeyPattern res = new DatabaseBibtexKeyPattern(Globals.prefs.getGlobalBibtexKeyPattern());
fillPatternUsingPanelData(res);
return res;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public GenerateBibtexKeySingleAction(BibEntry entry, BibDatabaseContext database
this.preferencesService = preferencesService;
this.undoManager = undoManager;

if (preferencesService.getBibtexKeyPatternPreferences().avoidOverwritingCiteKey()) {
if (preferencesService.getBibtexKeyPatternPreferences().shouldAvoidOverwriteCiteKey()) {
this.executable.bind(entry.getCiteKeyBinding().isEmpty());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public BibtexKeyPatternTabView(JabRefPreferences preferences) {

bibtexKeyPatternTable = new BibtexKeyPatternPanel(preferences,
Globals.entryTypesManager.getAllTypes(preferences.getDefaultBibDatabaseMode()),
preferences.getKeyPattern());
preferences.getGlobalBibtexKeyPattern());

ViewLoader.view(this)
.root(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,46 +16,51 @@
import org.jabref.gui.DialogService;
import org.jabref.gui.commonfxcontrols.BibtexKeyPatternPanelItemModel;
import org.jabref.gui.commonfxcontrols.BibtexKeyPatternPanelViewModel;
import org.jabref.logic.bibtexkeypattern.BibtexKeyPatternPreferences;
import org.jabref.model.bibtexkeypattern.GlobalBibtexKeyPattern;
import org.jabref.preferences.JabRefPreferences;
import org.jabref.preferences.PreferencesService;

public class BibtexKeyPatternTabViewModel implements PreferenceTabViewModel {

private BooleanProperty overwriteAllowProperty = new SimpleBooleanProperty();
private BooleanProperty overwriteWarningProperty = new SimpleBooleanProperty();
private BooleanProperty generateOnSaveProperty = new SimpleBooleanProperty();
private BooleanProperty letterStartAProperty = new SimpleBooleanProperty();
private BooleanProperty letterStartBProperty = new SimpleBooleanProperty();
private BooleanProperty letterAlwaysAddProperty = new SimpleBooleanProperty();
private StringProperty keyPatternRegexProperty = new SimpleStringProperty();
private StringProperty keyPatternReplacementProperty = new SimpleStringProperty();
private StringProperty unwantedCharactersProperty = new SimpleStringProperty();
private final BooleanProperty overwriteAllowProperty = new SimpleBooleanProperty();
private final BooleanProperty overwriteWarningProperty = new SimpleBooleanProperty();
private final BooleanProperty generateOnSaveProperty = new SimpleBooleanProperty();
private final BooleanProperty letterStartAProperty = new SimpleBooleanProperty();
private final BooleanProperty letterStartBProperty = new SimpleBooleanProperty();
private final BooleanProperty letterAlwaysAddProperty = new SimpleBooleanProperty();
private final StringProperty keyPatternRegexProperty = new SimpleStringProperty();
private final StringProperty keyPatternReplacementProperty = new SimpleStringProperty();
private final StringProperty unwantedCharactersProperty = new SimpleStringProperty();

// The list and the default properties are being overwritten by the bound properties of the tableView, but to
// prevent an NPE on storing the preferences before lazy-loading of the setValues, they need to be initialized.
private ListProperty<BibtexKeyPatternPanelItemModel> patternListProperty = new SimpleListProperty<>(FXCollections.observableArrayList());
private ObjectProperty<BibtexKeyPatternPanelItemModel> defaultKeyPatternProperty = new SimpleObjectProperty<>(
private final ListProperty<BibtexKeyPatternPanelItemModel> patternListProperty = new SimpleListProperty<>(FXCollections.observableArrayList());
private final ObjectProperty<BibtexKeyPatternPanelItemModel> defaultKeyPatternProperty = new SimpleObjectProperty<>(
new BibtexKeyPatternPanelItemModel(new BibtexKeyPatternPanelViewModel.DefaultEntryType(), ""));

private final DialogService dialogService;
private final JabRefPreferences preferences;
private final PreferencesService preferences;
private final BibtexKeyPatternPreferences initialBibtexKeyPatternPreferences;

public BibtexKeyPatternTabViewModel(DialogService dialogService, JabRefPreferences preferences) {
public BibtexKeyPatternTabViewModel(DialogService dialogService, PreferencesService preferences) {
this.dialogService = dialogService;
this.preferences = preferences;
this.initialBibtexKeyPatternPreferences = preferences.getBibtexKeyPatternPreferences();
}

@Override
public void setValues() {
overwriteAllowProperty.setValue(!preferences.getBoolean(JabRefPreferences.AVOID_OVERWRITING_KEY));
overwriteWarningProperty.setValue(preferences.getBoolean(JabRefPreferences.WARN_BEFORE_OVERWRITING_KEY));
generateOnSaveProperty.setValue(preferences.getBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING));
overwriteAllowProperty.setValue(!initialBibtexKeyPatternPreferences.shouldAvoidOverwriteCiteKey());
overwriteWarningProperty.setValue(initialBibtexKeyPatternPreferences.shouldWarnBeforeOverwriteCiteKey());
generateOnSaveProperty.setValue(initialBibtexKeyPatternPreferences.shouldGenerateCiteKeysBeforeSaving());

if (preferences.getBoolean(JabRefPreferences.KEY_GEN_ALWAYS_ADD_LETTER)) {
if (initialBibtexKeyPatternPreferences.getKeySuffix()
== BibtexKeyPatternPreferences.KeySuffix.ALWAYS) {
letterAlwaysAddProperty.setValue(true);
letterStartAProperty.setValue(false);
letterStartBProperty.setValue(false);
} else if (preferences.getBoolean(JabRefPreferences.KEY_GEN_FIRST_LETTER_A)) {
} else if (initialBibtexKeyPatternPreferences.getKeySuffix()
== BibtexKeyPatternPreferences.KeySuffix.SECOND_WITH_A) {
letterAlwaysAddProperty.setValue(false);
letterStartAProperty.setValue(true);
letterStartBProperty.setValue(false);
Expand All @@ -65,38 +70,15 @@ public void setValues() {
letterStartBProperty.setValue(true);
}

keyPatternRegexProperty.setValue(preferences.get(JabRefPreferences.KEY_PATTERN_REGEX));
keyPatternReplacementProperty.setValue(preferences.get(JabRefPreferences.KEY_PATTERN_REPLACEMENT));
unwantedCharactersProperty.setValue(preferences.get(JabRefPreferences.UNWANTED_BIBTEX_KEY_CHARACTERS));
keyPatternRegexProperty.setValue(initialBibtexKeyPatternPreferences.getKeyPatternRegex());
keyPatternReplacementProperty.setValue(initialBibtexKeyPatternPreferences.getKeyPatternReplacement());
unwantedCharactersProperty.setValue(initialBibtexKeyPatternPreferences.getUnwantedCharacters());
}

@Override
public void storeSettings() {
preferences.put(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN, defaultKeyPatternProperty.getValue().getPattern());
preferences.putBoolean(JabRefPreferences.AVOID_OVERWRITING_KEY, !overwriteAllowProperty.getValue());
preferences.putBoolean(JabRefPreferences.WARN_BEFORE_OVERWRITING_KEY, overwriteWarningProperty.getValue());
preferences.putBoolean(JabRefPreferences.GENERATE_KEYS_BEFORE_SAVING, generateOnSaveProperty.getValue());

if (letterAlwaysAddProperty.getValue()) {
preferences.putBoolean(JabRefPreferences.KEY_GEN_ALWAYS_ADD_LETTER, true);
preferences.putBoolean(JabRefPreferences.KEY_GEN_FIRST_LETTER_A, false);
} else if (letterStartAProperty.getValue()) {
preferences.putBoolean(JabRefPreferences.KEY_GEN_ALWAYS_ADD_LETTER, false);
preferences.putBoolean(JabRefPreferences.KEY_GEN_FIRST_LETTER_A, true);
} else if (letterStartBProperty.getValue()) {
preferences.putBoolean(JabRefPreferences.KEY_GEN_ALWAYS_ADD_LETTER, false);
preferences.putBoolean(JabRefPreferences.KEY_GEN_FIRST_LETTER_A, false);
} else {
// No Radioitem selected, should not happen, but if, make KEY_GEN_FIRST_LETTER_A default
preferences.putBoolean(JabRefPreferences.KEY_GEN_ALWAYS_ADD_LETTER, false);
preferences.putBoolean(JabRefPreferences.KEY_GEN_FIRST_LETTER_A, true);
}

preferences.put(JabRefPreferences.KEY_PATTERN_REGEX, keyPatternRegexProperty.getValue());
preferences.put(JabRefPreferences.KEY_PATTERN_REPLACEMENT, keyPatternReplacementProperty.getValue());
preferences.put(JabRefPreferences.UNWANTED_BIBTEX_KEY_CHARACTERS, unwantedCharactersProperty.getValue());

GlobalBibtexKeyPattern newKeyPattern = GlobalBibtexKeyPattern.fromPattern(preferences.get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN));
GlobalBibtexKeyPattern newKeyPattern =
new GlobalBibtexKeyPattern(initialBibtexKeyPatternPreferences.getKeyPattern().getDefaultValue());
patternListProperty.forEach(item -> {
String patternString = item.getPattern();
if (!item.getEntryType().getName().equals("default")) {
Expand All @@ -111,7 +93,25 @@ public void storeSettings() {
// at the end of the pattern
newKeyPattern.setDefaultValue(defaultKeyPatternProperty.getValue().getPattern());
}
preferences.putKeyPattern(newKeyPattern);

BibtexKeyPatternPreferences.KeySuffix keySuffix = BibtexKeyPatternPreferences.KeySuffix.ALWAYS;

if (letterStartAProperty.getValue()) {
keySuffix = BibtexKeyPatternPreferences.KeySuffix.SECOND_WITH_A;
} else if (letterStartBProperty.getValue()) {
keySuffix = BibtexKeyPatternPreferences.KeySuffix.SECOND_WITH_B;
}

preferences.storeBibtexKeyPatternPreferences(new BibtexKeyPatternPreferences(
!overwriteAllowProperty.getValue(),
overwriteWarningProperty.getValue(),
generateOnSaveProperty.getValue(),
keySuffix,
keyPatternRegexProperty.getValue(),
keyPatternReplacementProperty.getValue(),
unwantedCharactersProperty.getValue(),
newKeyPattern,
initialBibtexKeyPatternPreferences.getKeywordDelimiter()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
<?import org.controlsfx.control.textfield.CustomPasswordField?>
<fx:root prefWidth="650.0" spacing="10.0" type="VBox"
xmlns="http://javafx.com/javafx/8.0.212" xmlns:fx="http://javafx.com/fxml/1"
fx:controller="org.jabref.gui.preferences.AdvancedTabView">
<Label styleClass="titleHeader" text="%Advanced"/>
fx:controller="org.jabref.gui.preferences.NetworkTabView">
<Label styleClass="titleHeader" text="%Network"/>
calixtus marked this conversation as resolved.
Show resolved Hide resolved
<Label styleClass="sectionHeader" text="%Remote operation"/>
<Label fx:id="remoteLabel"
text="%This feature lets new files be opened or imported into an already running instance of JabRef instead of opening a new instance. For instance, this is useful when you open a file in JabRef from your web browser. Note that this will prevent you from running more than one instance of JabRef at a time."
Expand All @@ -24,12 +24,7 @@
<Button fx:id="remoteHelp" prefWidth="20.0"/>
</HBox>

<Label styleClass="sectionHeader" text="%Import conversions"/>
<CheckBox fx:id="useCaseKeeper" text="%Add {} to specified title words on search to keep the correct case"/>
<CheckBox fx:id="useUnitFormatter"
text="%Format units by adding non-breaking separators and keeping the correct case on search"/>

<Label styleClass="sectionHeader" text="%Network"/>
<Label styleClass="sectionHeader" text="%Proxy configuration"/>
<GridPane maxHeight="-Infinity" maxWidth="-Infinity" hgap="10.0" vgap="10.0">
<columnConstraints>
<ColumnConstraints hgrow="SOMETIMES"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,11 @@
import de.saxsys.mvvmfx.utils.validation.visualization.ControlsFxVisualizer;
import org.controlsfx.control.textfield.CustomPasswordField;

public class AdvancedTabView extends AbstractPreferenceTabView<AdvancedTabViewModel> implements PreferencesTab {
public class NetworkTabView extends AbstractPreferenceTabView<NetworkTabViewModel> implements PreferencesTab {
@FXML private Label remoteLabel;
@FXML private CheckBox remoteServer;
@FXML private TextField remotePort;
@FXML private Button remoteHelp;
@FXML private CheckBox useCaseKeeper;
@FXML private CheckBox useUnitFormatter;

@FXML private CheckBox proxyUse;
@FXML private Label proxyHostnameLabel;
Expand All @@ -48,7 +46,7 @@ public class AdvancedTabView extends AbstractPreferenceTabView<AdvancedTabViewMo

private final ControlsFxVisualizer validationVisualizer = new ControlsFxVisualizer();

public AdvancedTabView(JabRefPreferences preferences) {
public NetworkTabView(JabRefPreferences preferences) {
this.preferences = preferences;

ViewLoader.view(this)
Expand All @@ -58,20 +56,17 @@ public AdvancedTabView(JabRefPreferences preferences) {

@Override
public String getTabName() {
return Localization.lang("Advanced");
return Localization.lang("Network");
}

public void initialize() {
this.viewModel = new AdvancedTabViewModel(dialogService, preferences);
this.viewModel = new NetworkTabViewModel(dialogService, preferences);

remoteLabel.setVisible(preferences.getBoolean(JabRefPreferences.SHOW_ADVANCED_HINTS));
remoteServer.selectedProperty().bindBidirectional(viewModel.remoteServerProperty());
remotePort.textProperty().bindBidirectional(viewModel.remotePortProperty());
remotePort.disableProperty().bind(remoteServer.selectedProperty().not());

useCaseKeeper.selectedProperty().bindBidirectional(viewModel.useCaseKeeperProperty());
useUnitFormatter.selectedProperty().bindBidirectional(viewModel.useUnitFormatterProperty());

proxyUse.selectedProperty().bindBidirectional(viewModel.proxyUseProperty());
proxyHostnameLabel.disableProperty().bind(proxyUse.selectedProperty().not());
proxyHostname.textProperty().bindBidirectional(viewModel.proxyHostnameProperty());
Expand Down
Loading