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

Change default behavior of resolve bibtex strings #8382

Merged
merged 33 commits into from
Jan 13, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
e69c84f
Change default behavior of resolve bibtex strings
Siedlerchr Jan 3, 2022
ae29ece
Merge remote-tracking branch 'upstream/main' into stringResolving
Siedlerchr Jan 4, 2022
30e6b54
Renaming of fields
Siedlerchr Jan 4, 2022
56f2f8a
fix prefs, remove migration
Siedlerchr Jan 4, 2022
fa84969
Fix gui properties and l10n
Siedlerchr Jan 4, 2022
ef2c916
adjust defaults, fix bst tests
Siedlerchr Jan 6, 2022
ba865d2
remove obsolete test
Siedlerchr Jan 6, 2022
8b2f76f
fix checkstyle
Siedlerchr Jan 6, 2022
7975ea4
fix another test
Siedlerchr Jan 6, 2022
adbb12d
fix comment
Siedlerchr Jan 7, 2022
3c81834
Fix typos
koppor Jan 7, 2022
d3ba9a1
Merge branch 'main' of github.com:JabRef/jabref into stringResolving
Siedlerchr Jan 8, 2022
3e264ef
Merge branch 'stringResolving' of github.com:JabRef/jabref into strin…
Siedlerchr Jan 8, 2022
df38fe9
add institution
Siedlerchr Jan 8, 2022
4dd29d3
Sort fields alphabetically
koppor Jan 9, 2022
3d54f61
Group prefernces at "File": BibTeX strings, Loading, Saving
koppor Jan 9, 2022
ce0a098
Merge branch 'main' into stringResolving
Siedlerchr Jan 9, 2022
adae72a
Remove unused imports
koppor Jan 9, 2022
83438f2
Add ADR-0024
koppor Jan 9, 2022
ec82fbb
Add test for comment field
Siedlerchr Jan 9, 2022
48dbc3a
Merge remote-tracking branch 'upstream/main' into stringResolving
Siedlerchr Jan 9, 2022
00ae093
fix tests
Siedlerchr Jan 9, 2022
97e6bb2
Fix markdown in ADR-0024
koppor Jan 9, 2022
2922009
ADR-0024: Fix filename and addr to adr.md
koppor Jan 9, 2022
ab7f578
ADR-0019: Fix typo
koppor Jan 9, 2022
fb4e8f9
Merge remote-tracking branch 'origin/main' into stringResolving
koppor Jan 9, 2022
3395f56
Remove obsolete empty line
koppor Jan 9, 2022
895d210
Introduce BIBTEX_STRING_START_END_SYMBOL and remove negation
koppor Jan 9, 2022
21ab02f
Remove deprecated constructor FieldWriterPreferences()
koppor Jan 9, 2022
242a203
Fix test on Windows (CRLF issue)
koppor Jan 9, 2022
beda8ab
Add missing context information
koppor Jan 9, 2022
6c56ad4
Add more tests
koppor Jan 9, 2022
1f38378
Fix negation
koppor Jan 9, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ Example:
left: BibTeX; right: Entry Editor

```text
@String{ value = "example" }

field1 = value -> #value#
field2 = {value} -> value

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public void setValues() {
openLastStartupProperty.setValue(preferences.shouldOpenLastFilesOnStartup());
noWrapFilesProperty.setValue(importExportPreferences.getNonWrappableFields());

doNotResolveStringsProperty.setValue(importExportPreferences.doNotResolveStrings());
resolveStringsProperty.setValue(!importExportPreferences.doNotResolveStrings());
doNotResolveStringsProperty.setValue(!importExportPreferences.resolveStrings());
resolveStringsProperty.setValue(!importExportPreferences.resolveStrings());
resolveStringsForFieldsProperty.setValue(importExportPreferences.getResolvableFields());
alwaysReformatBibProperty.setValue(importExportPreferences.shouldAlwaysReformatOnSave());
autosaveLocalLibraries.setValue(preferences.shouldAutosave());
Expand All @@ -43,7 +43,7 @@ public void setValues() {
public void storeSettings() {
preferences.storeOpenLastFilesOnStartup(openLastStartupProperty.getValue());

importExportPreferences.setDoNotResolveStrings(doNotResolveStringsProperty.getValue());
importExportPreferences.setResolveStrings(!doNotResolveStringsProperty.getValue());
importExportPreferences.setNonWrappableFields(noWrapFilesProperty.getValue().trim());
importExportPreferences.setResolvableFields(resolveStringsForFieldsProperty.getValue().trim());
importExportPreferences.setAlwaysReformatOnSave(alwaysReformatBibProperty.getValue());
Expand Down
19 changes: 12 additions & 7 deletions src/main/java/org/jabref/logic/bibtex/FieldWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
*/
public class FieldWriter {

// See also ADR-0024
public static final char BIBTEX_STRING_START_END_SYMBOL = '#';

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

private static final char FIELD_START = '{';
Expand Down Expand Up @@ -84,7 +87,6 @@ public String write(Field field, String content) throws InvalidFieldValueExcepti
return FIELD_START + String.valueOf(FIELD_END);
}

// If the field is non-standard, we will just append braces, wrap and write.
if (!shouldResolveStrings(field)) {
return formatWithoutResolvingStrings(content, field);
}
Expand All @@ -111,7 +113,7 @@ private String formatAndResolveStrings(String content, Field field) throws Inval
int goFrom = pivot;
int pos1 = pivot;
while (goFrom == pos1) {
pos1 = content.indexOf('#', goFrom);
pos1 = content.indexOf(BIBTEX_STRING_START_END_SYMBOL, goFrom);
if ((pos1 > 0) && (content.charAt(pos1 - 1) == '\\')) {
goFrom = pos1 + 1;
pos1++;
Expand All @@ -125,16 +127,19 @@ private String formatAndResolveStrings(String content, Field field) throws Inval
pos1 = content.length(); // No more occurrences found.
pos2 = -1;
} else {
pos2 = content.indexOf('#', pos1 + 1);
pos2 = content.indexOf(BIBTEX_STRING_START_END_SYMBOL, pos1 + 1);
if (pos2 == -1) {
if (neverFailOnHashes) {
pos1 = content.length(); // just write out the rest of the text, and throw no exception
} else {
LOGGER.error("The # character is not allowed in BibTeX strings unless escaped as in '\\#'. "
LOGGER.error("The character {} is not allowed in BibTeX strings unless escaped as in '\\{}'. "
+ "In JabRef, use pairs of # characters to indicate a string. "
+ "Note that the entry causing the problem has been selected. Field value: {}", content);
+ "Note that the entry causing the problem has been selected. Field value: {}",
BIBTEX_STRING_START_END_SYMBOL,
BIBTEX_STRING_START_END_SYMBOL,
content);
throw new InvalidFieldValueException(
"The # character is not allowed in BibTeX strings unless escaped as in '\\#'.\n"
"The character " + BIBTEX_STRING_START_END_SYMBOL + " is not allowed in BibTeX strings unless escaped as in '\\" + BIBTEX_STRING_START_END_SYMBOL + "'.\n"
+ "In JabRef, use pairs of # characters to indicate a string.\n"
+ "Note that the entry causing the problem has been selected. Field value: " + content);
}
Expand Down Expand Up @@ -163,7 +168,7 @@ private String formatAndResolveStrings(String content, Field field) throws Inval
}

private boolean shouldResolveStrings(Field field) {
if (!preferences.isDoNotResolveStrings()) {
if (preferences.isResolveStrings()) {
Copy link
Member

Choose a reason for hiding this comment

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

shouldResolveStrings?

Copy link
Member

Choose a reason for hiding this comment

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

Follow-up PR 😇

// Resolve strings for the list of fields only
return preferences.getResolveStringsForFields().contains(field);
}
Expand Down
25 changes: 8 additions & 17 deletions src/main/java/org/jabref/logic/bibtex/FieldWriterPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,26 @@
import java.util.List;

import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.StandardField;

public class FieldWriterPreferences {

private final boolean doNotResolveStrings;
private final boolean resolveStrings;
private final List<Field> resolveStringsForFields;
private final int lineLength = 65; // Constant
private final FieldContentFormatterPreferences fieldContentFormatterPreferences;

public FieldWriterPreferences(boolean doNotResolveFields, List<Field> resolveStringsForFields,
/**
* @param resolveStrings true - The character {@link FieldWriter#BIBTEX_STRING_START_END_SYMBOL} should be interpreted as indicator of BibTeX strings
*/
public FieldWriterPreferences(boolean resolveStrings, List<Field> resolveStringsForFields,
FieldContentFormatterPreferences fieldContentFormatterPreferences) {
this.doNotResolveStrings = doNotResolveFields;
this.resolveStrings = resolveStrings;
this.resolveStringsForFields = resolveStringsForFields;
this.fieldContentFormatterPreferences = fieldContentFormatterPreferences;
}

/**
* ONLY USE IN TEST: ONLY resolves BibTex String for the Field Month
* Creates an instance with default values (not obeying any user preferences). This constructor should be used with
* caution. The other constructor has to be preferred.
* @deprecated
*/
@Deprecated
public FieldWriterPreferences() {
this(false, List.of(StandardField.MONTH), new FieldContentFormatterPreferences());
}

public boolean isDoNotResolveStrings() {
return doNotResolveStrings;
public boolean isResolveStrings() {
return resolveStrings;
}

public List<Field> getResolveStringsForFields() {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/logic/bst/VM.java
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ public String run(Collection<BibEntry> bibEntries, BibDatabase bibDatabase) {
* @param bibDatabase
*/
private void read(BibDatabase bibDatabase) {
FieldWriter fieldWriter = new FieldWriter(new FieldWriterPreferences(false, List.of(StandardField.MONTH), new FieldContentFormatterPreferences()));
FieldWriter fieldWriter = new FieldWriter(new FieldWriterPreferences(true, List.of(StandardField.MONTH), new FieldContentFormatterPreferences()));
for (BstEntry e : entries) {
for (Map.Entry<String, String> mEntry : e.fields.entrySet()) {
Field field = FieldFactory.parseField(mEntry.getKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.regex.Pattern;

import org.jabref.logic.bibtex.FieldContentFormatter;
import org.jabref.logic.bibtex.FieldWriter;
import org.jabref.logic.exporter.BibtexDatabaseWriter;
import org.jabref.logic.exporter.SavePreferences;
import org.jabref.logic.importer.ImportFormatPreferences;
Expand Down Expand Up @@ -641,14 +642,17 @@ private String parseFieldContent(Field field) throws IOException {
String number = parseTextToken();
value.append(number);
} else if (character == '#') {
// Here, we hit the case of BibTeX string concatenation. E.g., "author = Kopp # Kolb".
// We did NOT hit org.jabref.logic.bibtex.FieldWriter#BIBTEX_STRING_START_END_SYMBOL
// See also ADR-0024
consume('#');
} else {
String textToken = parseTextToken();
if (textToken.isEmpty()) {
throw new IOException("Error in line " + line + " or above: "
+ "Empty text token.\nThis could be caused " + "by a missing comma between two fields.");
}
value.append('#').append(textToken).append('#');
value.append(FieldWriter.BIBTEX_STRING_START_END_SYMBOL).append(textToken).append(FieldWriter.BIBTEX_STRING_START_END_SYMBOL);
}
skipWhitespace();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.jabref.logic.bibtex.FieldWriter;
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;

/**
* Checks, if there is an even number of unescaped #
* Checks, if there is an even number of unescaped # (FieldWriter.BIBTEX_STRING_START_END_SYMBOL)
*/
public class BibStringChecker implements EntryChecker {

// Detect # if it doesn't have a \ in front of it or if it starts the string
private static final Pattern UNESCAPED_HASH = Pattern.compile("(?<!\\\\)#|^#");
// Detect FieldWriter.BIBTEX_STRING_START_END_SYMBOL (#) if it doesn't have a \ in front of it or if it starts the string
private static final Pattern UNESCAPED_HASH = Pattern.compile("(?<!\\\\)" + FieldWriter.BIBTEX_STRING_START_END_SYMBOL + "|^" + FieldWriter.BIBTEX_STRING_START_END_SYMBOL);

@Override
public List<IntegrityMessage> check(BibEntry entry) {
Expand All @@ -33,6 +34,7 @@ public List<IntegrityMessage> check(BibEntry entry) {
hashCount++;
}
if ((hashCount & 1) == 1) { // Check if odd
// # is FieldWriter.BIBTEX_STRING_START_END_SYMBOL
results.add(new IntegrityMessage(Localization.lang("odd number of unescaped '#'"), entry,
field.getKey()));
}
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/jabref/model/database/BibDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;

import org.jabref.logic.bibtex.FieldWriter;
import org.jabref.model.database.event.EntriesAddedEvent;
import org.jabref.model.database.event.EntriesRemovedEvent;
import org.jabref.model.entry.BibEntry;
Expand Down Expand Up @@ -502,14 +503,14 @@ private String resolveContent(String result, Set<String> usedIds, Set<String> al
StringBuilder newRes = new StringBuilder();
int piv = 0;
int next;
while ((next = res.indexOf('#', piv)) >= 0) {
while ((next = res.indexOf(FieldWriter.BIBTEX_STRING_START_END_SYMBOL, piv)) >= 0) {

// We found the next string ref. Append the text
// up to it.
if (next > 0) {
newRes.append(res, piv, next);
}
int stringEnd = res.indexOf('#', next + 1);
int stringEnd = res.indexOf(FieldWriter.BIBTEX_STRING_START_END_SYMBOL, next + 1);
if (stringEnd >= 0) {
// We found the boundaries of the string ref,
// now resolve that one.
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/jabref/model/entry/Month.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Optional;

import org.jabref.logic.bibtex.FieldWriter;
import org.jabref.model.strings.StringUtil;

/**
Expand Down Expand Up @@ -165,7 +166,7 @@ public String getShortName() {
}

/**
* Returns the month in JabRef format. The format is the short 3-digit name surrounded by a '#'.
* Returns the month in JabRef format. The format is the short 3-digit name surrounded by a '#' (FieldWriter.BIBTEX_STRING_START_END_SYMBOL).
* Example: #jan#, #feb#, etc.
* <p>
* See <a href="https://github.com/JabRef/jabref/issues/263#issuecomment-151246595">Issue 263</a> for a discussion on that thing.
Expand All @@ -175,7 +176,7 @@ public String getShortName() {
* @return Month in JabRef format
*/
public String getJabRefFormat() {
return String.format("#%s#", shortName);
return String.format(FieldWriter.BIBTEX_STRING_START_END_SYMBOL + "%s" + FieldWriter.BIBTEX_STRING_START_END_SYMBOL, shortName);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/jabref/model/strings/StringUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.regex.Pattern;

import org.jabref.architecture.ApacheCommonsLang3Allowed;
import org.jabref.logic.bibtex.FieldWriter;

import com.google.common.base.CharMatcher;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -323,7 +324,7 @@ public static String putBracesAroundCapitals(String s) {
inBrace++;
} else if (c == '}') {
inBrace--;
} else if (!escaped && (c == '#')) {
} else if (!escaped && (c == FieldWriter.BIBTEX_STRING_START_END_SYMBOL)) {
inString = !inString;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@
public class ImportExportPreferences {

private final StringProperty nonWrappableFields;
private final BooleanProperty doNotResolveStrings;
private final BooleanProperty resolveStrings;
private final StringProperty resolvableFields;
private final BooleanProperty alwaysReformatOnSave;
private final ObjectProperty<Path> importWorkingDirectory;
private final StringProperty lastExportExtension;
private final ObjectProperty<Path> exportWorkingDirectory;

public ImportExportPreferences(String nonWrappableFields,
boolean doNotResolveStrings,
boolean resolveStrings,
String resolvableFields,
boolean alwaysReformatOnSave,
Path importWorkingDirectory,
String lastExportExtension,
Path exportWorkingDirectory) {
this.nonWrappableFields = new SimpleStringProperty(nonWrappableFields);
this.doNotResolveStrings = new SimpleBooleanProperty(doNotResolveStrings);
this.resolveStrings = new SimpleBooleanProperty(resolveStrings);
this.resolvableFields = new SimpleStringProperty(resolvableFields);
this.alwaysReformatOnSave = new SimpleBooleanProperty(alwaysReformatOnSave);
this.importWorkingDirectory = new SimpleObjectProperty<>(importWorkingDirectory);
Expand All @@ -47,16 +47,16 @@ public void setNonWrappableFields(String nonWrappableFields) {
this.nonWrappableFields.set(nonWrappableFields);
}

public boolean doNotResolveStrings() {
return doNotResolveStrings.get();
public boolean resolveStrings() {
return resolveStrings.get();
}

public BooleanProperty doNotResolveStringsProperty() {
return doNotResolveStrings;
public BooleanProperty resolveStringsProperty() {
return resolveStrings;
}

public void setDoNotResolveStrings(boolean doNotResolveStrings) {
this.doNotResolveStrings.set(doNotResolveStrings);
public void setResolveStrings(boolean resolveStrings) {
this.resolveStrings.set(resolveStrings);
}

public String getResolvableFields() {
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,7 @@ public FieldContentFormatterPreferences getFieldContentParserPreferences() {
@Override
public FieldWriterPreferences getFieldWriterPreferences() {
return new FieldWriterPreferences(
getBoolean(DO_NOT_RESOLVE_STRINGS),
!getBoolean(DO_NOT_RESOLVE_STRINGS),
getStringList(RESOLVE_STRINGS_FOR_FIELDS).stream().map(FieldFactory::parseField).collect(Collectors.toList()),
getFieldContentParserPreferences());
}
Expand Down Expand Up @@ -2294,15 +2294,15 @@ public ImportExportPreferences getImportExportPreferences() {

importExportPreferences = new ImportExportPreferences(
get(NON_WRAPPABLE_FIELDS),
getBoolean(DO_NOT_RESOLVE_STRINGS),
!getBoolean(DO_NOT_RESOLVE_STRINGS),
get(RESOLVE_STRINGS_FOR_FIELDS),
getBoolean(REFORMAT_FILE_ON_SAVE_AND_EXPORT),
Path.of(get(IMPORT_WORKING_DIRECTORY)),
get(LAST_USED_EXPORT),
Path.of(get(EXPORT_WORKING_DIRECTORY)));

EasyBind.listen(importExportPreferences.nonWrappableFieldsProperty(), (obs, oldValue, newValue) -> put(NON_WRAPPABLE_FIELDS, newValue));
EasyBind.listen(importExportPreferences.doNotResolveStringsProperty(), (obs, oldValue, newValue) -> putBoolean(DO_NOT_RESOLVE_STRINGS, newValue));
EasyBind.listen(importExportPreferences.resolveStringsProperty(), (obs, oldValue, newValue) -> putBoolean(DO_NOT_RESOLVE_STRINGS, !newValue));
EasyBind.listen(importExportPreferences.resolvableFieldsProperty(), (obs, oldValue, newValue) -> put(RESOLVE_STRINGS_FOR_FIELDS, newValue));
EasyBind.listen(importExportPreferences.alwaysReformatOnSaveProperty(), (obs, oldValue, newValue) -> putBoolean(REFORMAT_FILE_ON_SAVE_AND_EXPORT, newValue));
EasyBind.listen(importExportPreferences.importWorkingDirectoryProperty(), (obs, oldValue, newValue) -> put(IMPORT_WORKING_DIRECTORY, newValue.toString()));
Expand Down
5 changes: 4 additions & 1 deletion src/test/java/org/jabref/gui/entryeditor/SourceTabTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jabref.gui.entryeditor;

import java.util.Collections;
import java.util.List;

import javafx.scene.Scene;
import javafx.scene.control.Label;
Expand All @@ -18,6 +19,7 @@
import org.jabref.logic.importer.ImportFormatPreferences;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.field.UnknownField;
import org.jabref.model.util.DummyFileUpdateMonitor;
import org.jabref.testutils.category.GUITest;
Expand Down Expand Up @@ -52,8 +54,9 @@ public void onStart(Stage stage) {
ImportFormatPreferences importFormatPreferences = mock(ImportFormatPreferences.class);
when(importFormatPreferences.getFieldContentFormatterPreferences())
.thenReturn(mock(FieldContentFormatterPreferences.class));
FieldWriterPreferences fieldWriterPreferences = new FieldWriterPreferences(true, List.of(StandardField.MONTH), new FieldContentFormatterPreferences());

sourceTab = new SourceTab(new BibDatabaseContext(), new CountingUndoManager(), new FieldWriterPreferences(), importFormatPreferences, new DummyFileUpdateMonitor(), mock(DialogService.class), stateManager, keyBindingRepository);
sourceTab = new SourceTab(new BibDatabaseContext(), new CountingUndoManager(), fieldWriterPreferences, importFormatPreferences, new DummyFileUpdateMonitor(), mock(DialogService.class), stateManager, keyBindingRepository);
pane = new TabPane(
new Tab("main area", area),
new Tab("other tab", new Label("some text")),
Expand Down
Loading