Skip to content

Commit

Permalink
Change default behavior of resolve bibtex strings (#8382)
Browse files Browse the repository at this point in the history
* Change default behavior of resolve bibtex strings

Fixes #7010
Fixes #7012
Fixes #8303

* Renaming of fields

* fix prefs, remove migration

* Fix gui properties and l10n

* adjust defaults, fix bst tests

* remove obsolete test
Fix changelog

* fix checkstyle

* fix another test

* fix comment

* Fix typos

* add institution

* Sort fields alphabetically

* Group prefernces at "File": BibTeX strings, Loading, Saving

* Remove unused imports

* Add ADR-0024

* Add test for comment field

* fix tests

* Fix markdown in ADR-0024

* ADR-0024: Fix filename and addr to adr.md

* ADR-0019: Fix typo

* Remove obsolete empty line

* Introduce BIBTEX_STRING_START_END_SYMBOL and remove negation

- Add org.jabref.logic.bibtex.FieldWriter#BIBTEX_STRING_START_END_SYMBOL
- !doNotResolveStrings -> resolveStrings

* Remove deprecated constructor FieldWriterPreferences()

* Fix test on Windows (CRLF issue)

* Add missing context information

* Add more tests

* Fix negation

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
  • Loading branch information
Siedlerchr and koppor authored Jan 13, 2022
1 parent 9b8f17b commit ccf20b4
Show file tree
Hide file tree
Showing 23 changed files with 335 additions and 145 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
### Changed

- We integrated the external file types dialog directly inside the preferences. [#8341](https://github.com/JabRef/jabref/pull/8341)
- We inverted the logic for resolving [BibTeX strings](https://docs.jabref.org/advanced/strings). This helps to keep `#` chars. By default String resolving is only activated for a couple of standard fields. The list of fields can be modified in the preferences. [#7010](https://github.com/JabRef/jabref/issues/7010), [#7102](https://github.com/JabRef/jabref/issues/7012), [#8303](https://github.com/JabRef/jabref/issues/8303)
- We moved the search box in preview preferences closer to the available citation styles list. [#8370](https://github.com/JabRef/jabref/pull/8370)
- Changing the preference to show the preview panel as a separate tab now has effect without restarting JabRef. [#8370](https://github.com/JabRef/jabref/pull/8370)
- We enabled switching themes in JabRef without the need to restart JabRef. [#7335](https://github.com/JabRef/jabref/pull/7335)

### Fixed

- We fixed an issue where `#`chars in certain fields would be interpreted as BibTeX strings [#7010](https://github.com/JabRef/jabref/issues/7010), [#7102](https://github.com/JabRef/jabref/issues/7012), [#8303](https://github.com/JabRef/jabref/issues/8303)
- We fixed an issue where the fulltext search on an empty library with no documents would lead to an exception [koppor#522](https://github.com/koppor/jabref/issues/522)
- We fixed an issue where clicking on "Accept changes" in the merge dialog would lead to an exception [forum#2418](https://discourse.jabref.org/t/the-library-has-been-modified-by-another-program/2418/8)
- We fixed an issue where clicking on headings in the entry preview could lead to an exception. [#8292](https://github.com/JabRef/jabref/issues/8292)
Expand Down
3 changes: 2 additions & 1 deletion docs/adr.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ Architectural decisions for JabRef:
* [ADR-0016](https://github.com/JabRef/jabref/tree/main/docs/adr/0016-mutable-preferences-objects.md) - Mutable preferences objects
* [ADR-0017](https://github.com/JabRef/jabref/tree/main/docs/adr/0017-allow-model-access-logic.md) - Allow org.jabref.model to access org.jabref.logic
* [ADR-0018](https://github.com/JabRef/jabref/blob/main/docs/adr/0018-use-regular-expression-to-split-multiple-sentence-titles.md) - Use regular expression to split multiple-sentence titles
* [ADR-0019](https://github.com/JabRef/jabref/blob/main/docs/adr/0019-implement-special-fields-as-seperate-fields.md) - Implement special fields as seperate fields
* [ADR-0019](https://github.com/JabRef/jabref/blob/main/docs/adr/0019-implement-special-fields-as-seperate-fields.md) - Implement special fields as separate fields
* [ADR-0020](https://github.com/JabRef/jabref/blob/main/docs/adr/0020-use-Jackson-to-parse-study-yml.md) - Use Jackson to parse study.yml
* [ADR-0021](https://github.com/JabRef/jabref/blob/main/docs/adr/0021-keep-study-as-a-dto.md) - Keep study as a DTO
* [ADR-0022](https://github.com/JabRef/jabref/blob/main/docs/adr/0022-remove-stop-words-during-query-transformation.md) - Remove stop words during query transformation
* [ADR-0023](https://github.com/JabRef/jabref/blob/main/docs/adr/0023-localized-preferences.md) - Localized Preferences
* [ADR-0024](https://github.com/JabRef/jabref/blob/main/docs/adr/0024-use-#-as-indicator-for-BibTeX-string-constants.md) - Use `#` as indicator for BibTeX string constants

For new ADRs, please use [template.md](https://github.com/JabRef/jabref/tree/main/docs/adr/template.md) as basis. More information on the used format is available at [https://adr.github.io/madr/](https://adr.github.io/madr/). General information about architectural decision records is available at [https://adr.github.io/](https://adr.github.io/). Then add them to the above list.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Implement special fields as seperate fields
# Implement special fields as separate fields

## Context and Problem Statement

Expand Down
133 changes: 133 additions & 0 deletions docs/adr/0024-use-#-as-indicator-for-BibTeX-string-constants.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# Use `#` as indicator for BibTeX string constants

Bibtex supports string constants. The entry editor should support that, too.
Affected code is (at least) `org.jabref.logic.importer.fileformat.BibtexParser#parseFieldContent` and `org.jabref.logic.bibtex.FieldWriter#formatAndResolveStrings`

## Decision Drivers

* Full BibTeX compatibility
* Intuitive for the user
* UI backwards compatible

## Considered Options

* Wrap string constants in `#`
* Replace the internal `#` in JabRef by the non-used character `§`
* Replace the internal `#` in JabRef by the non-used character `%`
* Replace the internal `#` in JabRef by the non-used character `&`
* Replace the internal `#` in JabRef by a single `&`
* Change the data type of a field value
* Wrap plan # in curly braces: `{#}`

## Decision Outcome

Chosen option: "Wrap string constants in #", because already existing behavior

## Pros and Cons of the Options

### Wrap string constants in `#`

When BibTeX on the disk contains `field = value`, it is rendered as `field = #value#` in the entry editor. The user then knows that `value` is a BibTeX string.

Example:

left: BibTeX; right: Entry Editor

```text
@String{ value = "example" }
field1 = value -> #value#
field2 = {value} -> value
field1 = value # value --> #value# # #value#
```

* Good, because In place in JabRef since more than 10 years
* Bad, because `#` is used in BibTeX for string concatenation
* Bad, because `#` is used in Markdown to indicate headings. When using Markdown in fields such as comments, this causes writing errors
* Bad, because `#` could appear in URLs

### Replace the internal `#` in JabRef by the non-used character `§`

When BibTeX on the disk contains `field = value`, it is rendered as `field = §value§` in the entry editor. The user then knows that `value` is a BibTeX string.

Example:

left: BibTeX; right: Entry Editor

```text
field1 = value -> §value§
field2 = {value} -> value
field1 = value # value --> §value§ # §value§
```

* Good, because Easy to implement
* Good, because No conflict with BibTeX's `#` sign
* Bad, because Documentation needs to be updated
* Bad, because `§` is not available on an US keyboard

### Replace the internal `#` in JabRef by the non-used character `%`

Similar to "Replace the internal # in JabRef by the non-used character §"

Example:

left: BibTeX; right: Entry Editor

```text
field1 = value -> %value%
field2 = {value} -> value
field1 = value # value --> %value% # %value%
```

* Good, because A user does not write LaTeX comments inside a string
* Bad, because `%` could be used in Markdown, too
* Bad, because `%` is used for comments in LaTeX and thus migtht cause confusion
* Bad, because `%` could appear in URLs: There is [url percent encoding](https://www.w3schools.com/tags/ref_urlencode.asp)

### Replace the internal `#` in JabRef by the non-used character `&`

Similar to "Replace the internal `#` in JabRef by the non-used character `§`"

Example:

left: BibTeX; right: Entry Editor

```text
field1 = value -> &value&
field2 = {value} -> value
field1 = value # value --> &value& # &value&
```

* Good, because Users do not write LaTeX tables
* Bad, because `&` is a LaTeX command for tables

### Replace the internal `#` in JabRef by a single `&`

Prefix strings with `&`

Example:

left: BibTeX; right: Entry Editor

```text
field1 = value -> &value
field2 = {value} -> value
field1 = value # value --> &value # &value
```

* Good, because Near to C syntax
* Bad, because `&` is a LaTeX command for tables
* Bad, because `&` could appear in URLs

### Change the data type of a field value

`org.jabref.model.entry.BibEntry#setField(org.jabref.model.entry.field.Field, java.lang.String)` changes to `org.jabref.model.entry.BibEntry#setField(org.jabref.model.entry.field.Field, org.jabref.model.entry.field.Value)` (org.jabref.model.entry.BibEntry#setField(org.jabref.model.entry.field.Field, java.lang.String)). This would bring JabRef's internal data model even more close to BibTeX

* Good, because More object-orientated design of BibTeX field storage
* Good, because Easies implementation of an advanced field editor
* Bad, because High effort to implement
27 changes: 16 additions & 11 deletions src/main/java/org/jabref/gui/preferences/file/FileTab.fxml
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>


<?import javafx.scene.control.Button?>
<?import javafx.scene.control.CheckBox?>
<?import javafx.scene.control.ComboBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.RadioButton?>
<?import javafx.scene.control.TextField?>
Expand All @@ -15,27 +13,34 @@
xmlns="http://javafx.com/javafx" xmlns:fx="http://javafx.com/fxml"
fx:controller="org.jabref.gui.preferences.file.FileTab">
<Label text="%File" styleClass="titleHeader"/>

<Label styleClass="sectionHeader" text="%String constants"/>
<fx:define>
<ToggleGroup fx:id="stringsResolveToggleGroup"/>
</fx:define>
<HBox alignment="CENTER_LEFT" spacing="10.0">
<RadioButton fx:id="resolveStrings" text="%Resolve BibTeX strings for the following fields"
toggleGroup="$stringsResolveToggleGroup"/>
<TextField fx:id="resolveStringsForFields" HBox.hgrow="ALWAYS"/>
</HBox>
<RadioButton fx:id="doNotResolveStrings" text="%Do not resolve BibTeX strings"
toggleGroup="$stringsResolveToggleGroup"/>

<Label styleClass="sectionHeader" text="%Loading"/>

<CheckBox fx:id="openLastStartup" text="%Open last edited libraries at startup"/>

<Label styleClass="sectionHeader" text="%Saving"/>

<HBox alignment="CENTER_LEFT" spacing="10.0">
<Label text="%Do not wrap the following fields when saving"/>
<TextField fx:id="noWrapFiles" HBox.hgrow="ALWAYS"/>
</HBox>
<RadioButton fx:id="resolveStringsBibTex" text="%Resolve strings for standard BibTeX fields only"
toggleGroup="$stringsResolveToggleGroup"/>
<HBox alignment="CENTER_LEFT" spacing="10.0">
<RadioButton fx:id="resolveStringsAll" text="%Resolve strings for all fields except"
toggleGroup="$stringsResolveToggleGroup"/>
<TextField fx:id="resolveStringsExcept" HBox.hgrow="ALWAYS"/>
</HBox>
<CheckBox fx:id="alwaysReformatBib" text="%Always reformat BIB file on save and export"/>

<Label styleClass="sectionHeader" text="%Autosave"/>
<HBox alignment="CENTER_LEFT" spacing="10.0">
<CheckBox fx:id="autosaveLocalLibraries" text="%Autosave local libraries"/>
<Button fx:id="autosaveLocalLibrariesHelp"/>
</HBox>

<CheckBox fx:id="alwaysReformatBib" text="%Always reformat BIB file on save and export"/>
</fx:root>
17 changes: 9 additions & 8 deletions src/main/java/org/jabref/gui/preferences/file/FileTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ public class FileTab extends AbstractPreferenceTabView<FileTabViewModel> impleme

@FXML private CheckBox openLastStartup;
@FXML private TextField noWrapFiles;
@FXML private RadioButton resolveStringsBibTex;
@FXML private RadioButton resolveStringsAll;
@FXML private TextField resolveStringsExcept;
@FXML private RadioButton doNotResolveStrings;
@FXML private RadioButton resolveStrings;
@FXML private TextField resolveStringsForFields;
@FXML private CheckBox alwaysReformatBib;

@FXML private CheckBox autosaveLocalLibraries;
Expand All @@ -39,12 +39,13 @@ public void initialize() {
this.viewModel = new FileTabViewModel(preferencesService);
openLastStartup.selectedProperty().bindBidirectional(viewModel.openLastStartupProperty());
noWrapFiles.textProperty().bindBidirectional(viewModel.noWrapFilesProperty());
resolveStringsBibTex.selectedProperty().bindBidirectional(viewModel.resolveStringsBibTexProperty());
resolveStringsAll.selectedProperty().bindBidirectional(viewModel.resolveStringsAllProperty());
resolveStringsExcept.textProperty().bindBidirectional(viewModel.resolveStringsExceptProperty());
resolveStringsExcept.disableProperty().bind(resolveStringsAll.selectedProperty().not());
alwaysReformatBib.selectedProperty().bindBidirectional(viewModel.alwaysReformatBibProperty());

doNotResolveStrings.selectedProperty().bindBidirectional(viewModel.doNotResolveStringsProperty());
resolveStrings.selectedProperty().bindBidirectional(viewModel.resolveStringsProperty());
resolveStringsForFields.textProperty().bindBidirectional(viewModel.resolveStringsForFieldsProperty());
resolveStringsForFields.disableProperty().bind(doNotResolveStrings.selectedProperty());

alwaysReformatBib.selectedProperty().bindBidirectional(viewModel.alwaysReformatBibProperty());
autosaveLocalLibraries.selectedProperty().bindBidirectional(viewModel.autosaveLocalLibrariesProperty());

ActionFactory actionFactory = new ActionFactory(Globals.getKeyPrefs());
Expand Down
31 changes: 14 additions & 17 deletions src/main/java/org/jabref/gui/preferences/file/FileTabViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ public class FileTabViewModel implements PreferenceTabViewModel {

private final BooleanProperty openLastStartupProperty = new SimpleBooleanProperty();
private final StringProperty noWrapFilesProperty = new SimpleStringProperty("");
private final BooleanProperty resolveStringsBibTexProperty = new SimpleBooleanProperty();
private final BooleanProperty resolveStringsAllProperty = new SimpleBooleanProperty();
private final StringProperty resolveStringsExceptProperty = new SimpleStringProperty("");
private final BooleanProperty doNotResolveStringsProperty = new SimpleBooleanProperty();
private final BooleanProperty resolveStringsProperty = new SimpleBooleanProperty();
private final StringProperty resolveStringsForFieldsProperty = new SimpleStringProperty("");
private final BooleanProperty alwaysReformatBibProperty = new SimpleBooleanProperty();
private final BooleanProperty autosaveLocalLibraries = new SimpleBooleanProperty();

Expand All @@ -30,25 +30,22 @@ public class FileTabViewModel implements PreferenceTabViewModel {
@Override
public void setValues() {
openLastStartupProperty.setValue(preferences.shouldOpenLastFilesOnStartup());

noWrapFilesProperty.setValue(importExportPreferences.getNonWrappableFields());
resolveStringsAllProperty.setValue(importExportPreferences.shouldResolveStringsForAllStrings()); // Flipped around
resolveStringsBibTexProperty.setValue(importExportPreferences.shouldResolveStringsForStandardBibtexFields());
resolveStringsExceptProperty.setValue(importExportPreferences.getNonResolvableFields());

doNotResolveStringsProperty.setValue(!importExportPreferences.resolveStrings());
resolveStringsProperty.setValue(importExportPreferences.resolveStrings());
resolveStringsForFieldsProperty.setValue(importExportPreferences.getResolvableFields());
alwaysReformatBibProperty.setValue(importExportPreferences.shouldAlwaysReformatOnSave());

autosaveLocalLibraries.setValue(preferences.shouldAutosave());
}

@Override
public void storeSettings() {
preferences.storeOpenLastFilesOnStartup(openLastStartupProperty.getValue());

importExportPreferences.setResolveStrings(!doNotResolveStringsProperty.getValue());
importExportPreferences.setNonWrappableFields(noWrapFilesProperty.getValue().trim());
importExportPreferences.setResolveStringsForStandardBibtexFields(resolveStringsBibTexProperty.getValue());
importExportPreferences.setResolveStringsForAllStrings(resolveStringsAllProperty.getValue());
importExportPreferences.setNonResolvableFields(resolveStringsExceptProperty.getValue().trim());
importExportPreferences.setResolvableFields(resolveStringsForFieldsProperty.getValue().trim());
importExportPreferences.setAlwaysReformatOnSave(alwaysReformatBibProperty.getValue());

preferences.storeShouldAutosave(autosaveLocalLibraries.getValue());
Expand All @@ -66,16 +63,16 @@ public StringProperty noWrapFilesProperty() {
return noWrapFilesProperty;
}

public BooleanProperty resolveStringsBibTexProperty() {
return resolveStringsBibTexProperty;
public BooleanProperty doNotResolveStringsProperty() {
return doNotResolveStringsProperty;
}

public BooleanProperty resolveStringsAllProperty() {
return resolveStringsAllProperty;
public BooleanProperty resolveStringsProperty() {
return resolveStringsProperty;
}

public StringProperty resolveStringsExceptProperty() {
return resolveStringsExceptProperty;
public StringProperty resolveStringsForFieldsProperty() {
return resolveStringsForFieldsProperty;
}

public BooleanProperty alwaysReformatBibProperty() {
Expand Down
Loading

0 comments on commit ccf20b4

Please sign in to comment.