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

Remove dash from illegal characters. #6300

Merged
merged 32 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2b7fad5
Remove dash from illegal characters. Fix #6295 #6257 #6252
dextep Apr 16, 2020
f332e42
Update bibtexkey incompatible characters.
dextep Apr 17, 2020
38687aa
Add missing abbreviated journal names (#6292)
mayrmt Apr 16, 2020
feb6709
Change in CHANGELOG.md described.
dextep Apr 17, 2020
23d9720
Revert "Add missing abbreviated journal names (#6292)"
dextep Apr 17, 2020
a90ac60
Update CHANGELOG.md
dextep Apr 17, 2020
ed42ac1
Update CHANGELOG.md
dextep Apr 17, 2020
6fc9645
Renamed 'KEY_UNWANTED_CHARACTERS' to 'KEY_CHARACTERS_CAUSING_PARSING_…
dextep Apr 17, 2020
00f8c92
Merged illegal `KEY_ILLEGAL_CHARACTERS` and disallowed `KEY_CHARACTER…
dextep Apr 22, 2020
e39cca3
Adapted the tests according to code updates.
dextep Apr 23, 2020
471ec0e
Removed 'enforceLegalKey'.
dextep Apr 28, 2020
19e0879
Removed "Enforce legal characters in BibTeX keys" option in the prefe…
dextep May 1, 2020
2161cd1
Changes related to the review.
dextep May 2, 2020
ae8b183
Remove dash from illegal characters. Fix #6295 #6257 #6252
dextep Apr 16, 2020
08257ff
Update bibtexkey incompatible characters.
dextep Apr 17, 2020
d0e5241
Change in CHANGELOG.md described.
dextep Apr 17, 2020
8a510e3
Revert "Add missing abbreviated journal names (#6292)"
dextep Apr 17, 2020
d7d5ac7
Update CHANGELOG.md
dextep Apr 17, 2020
dda3259
Update CHANGELOG.md
dextep Apr 17, 2020
7362457
Renamed 'KEY_UNWANTED_CHARACTERS' to 'KEY_CHARACTERS_CAUSING_PARSING_…
dextep Apr 17, 2020
fe43020
Merged illegal `KEY_ILLEGAL_CHARACTERS` and disallowed `KEY_CHARACTER…
dextep Apr 22, 2020
aae2b0e
Adapted the tests according to code updates.
dextep Apr 23, 2020
4807493
Removed 'enforceLegalKey'.
dextep Apr 28, 2020
0b1ca1a
Removed "Enforce legal characters in BibTeX keys" option in the prefe…
dextep May 1, 2020
d64b911
Changes related to the review.
dextep May 2, 2020
6032766
Changes related to the review.
dextep May 3, 2020
995ff60
Merge branch 'remove-dash-from-illegal-chars' of https://github.com/d…
dextep May 3, 2020
4d37195
Merge remote-tracking branch 'upstream/master' into remove-dash-from-…
dextep May 4, 2020
3f5b2e5
Fix checkstyle.
dextep May 5, 2020
6db1344
Changes related to the review.
dextep May 5, 2020
c1ec4ad
Changes related to the review.
dextep May 6, 2020
cfeea59
Changes related to the review.
dextep May 6, 2020
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

### Added

- We added a new field in the preferences in 'BibTeX key generator' for unwanted characters that can be user-specified. [#6295](https://github.com/JabRef/jabref/issues/6295)
- We added support for searching ShortScience for an entry through the user's browser. [#6018](https://github.com/JabRef/jabref/pull/6018)
- We updated EditionChecker to permit edition to start with a number. [#6144](https://github.com/JabRef/jabref/issues/6144)
- We added tooltips for most fields in the entry editor containing a short description. [#5847](https://github.com/JabRef/jabref/issues/5847)
Expand All @@ -33,6 +34,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
### Fixed

- We fixed wrong button order (Apply and Cancel) in ManageProtectedTermsDialog.
- We fixed an issue with incompatible characters at BibTeX key [#6257](https://github.com/JabRef/jabref/issues/6257)
dextep marked this conversation as resolved.
Show resolved Hide resolved
- We fixed an issue where dash (`-`) was reported as illegal BibTeX key [#6295](https://github.com/JabRef/jabref/issues/6295)
dextep marked this conversation as resolved.
Show resolved Hide resolved
- We greatly improved the performance of the overall application and many operations. [#5071](https://github.com/JabRef/jabref/issues/5071)
- We fixed an issue where sort by priority was broken. [#6222](https://github.com/JabRef/jabref/issues/6222)
- We fixed an issue where opening a library from the recent libraries menu was not possible. [#5939](https://github.com/JabRef/jabref/issues/5939)
Expand All @@ -53,6 +56,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

### Removed

- We removed the option of the "enforce legal key". [#6295](https://github.com/JabRef/jabref/issues/6295)
- We removed the obsolete `External programs / Open PDF` section in the preferences, as the default application to open PDFs is now set in the `Manage external file types` dialog. [#6130](https://github.com/JabRef/jabref/pull/6130)
- We removed the option to configure whether a `.bib.bak` file should be generated upon save. It is now always enabled. Documentation at <https://docs.jabref.org/general/autosave>. [#6092](https://github.com/JabRef/jabref/issues/6092)
- We removed the built-in list of IEEE journal abbreviations using BibTeX strings. If you still want to use them, you have to download them separately from <https://abbrv.jabref.org>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void saveFields() {
return;
}

String testString = BibtexKeyGenerator.cleanKey(parts[1], preferences.getEnforceLegalKeys());
String testString = BibtexKeyGenerator.cleanKey(parts[1], preferences.getUnwantedCharacters());
if (!testString.equals(parts[1]) || (parts[1].indexOf('&') >= 0)) {
dialogService.showInformationDialogAndWait(
Localization.lang("Error"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public static FieldEditorFX getForField(final Field field,
databaseContext,
preferences.getFilePreferences(),
journalAbbreviationRepository,
preferences.getBoolean(JabRefPreferences.ENFORCE_LEGAL_BIBTEX_KEY),
preferences.getBoolean(JabRefPreferences.ALLOW_INTEGER_EDITION_BIBTEX));

final boolean isSingleLine = FieldFactory.isSingleLineField(field);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ public void execute() {
Globals.prefs.getFilePreferences(),
Globals.prefs.getBibtexKeyPatternPreferences(),
Globals.journalAbbreviationRepository,
Globals.prefs.getBoolean(JabRefPreferences.ENFORCE_LEGAL_BIBTEX_KEY),
Globals.prefs.getBoolean(JabRefPreferences.ALLOW_INTEGER_EDITION_BIBTEX));

Task<List<IntegrityMessage>> task = new Task<>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@
</padding>
</HBox>

<HBox alignment="CENTER_LEFT" spacing="10.0">
<Label text="%Remove the following characters:"/>
<TextField fx:id="unwantedCharacters" HBox.hgrow="ALWAYS"/>
</HBox>

<HBox prefWidth="650.0">
<Label styleClass="sectionHeader" text="%Key patterns"/>
<HBox HBox.hgrow="ALWAYS"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class BibtexKeyPatternTabView extends AbstractPreferenceTabView<BibtexKey
@FXML public RadioButton letterAlwaysAdd;
@FXML public TextField keyPatternRegex;
@FXML public TextField keyPatternReplacement;
@FXML public TextField unwantedCharacters;
@FXML public HBox keyPatternContainer;
@FXML public Button keyPatternHelp;

Expand Down Expand Up @@ -61,6 +62,7 @@ public void initialize() {
letterAlwaysAdd.selectedProperty().bindBidirectional(viewModel.letterAlwaysAddProperty());
keyPatternRegex.textProperty().bindBidirectional(viewModel.keyPatternRegexProperty());
keyPatternReplacement.textProperty().bindBidirectional(viewModel.keyPatternReplacementProperty());
unwantedCharacters.textProperty().bindBidirectional(viewModel.unwantedCharactersProperty());

bibtexKeyPatternTable.setPrefWidth(650.0);
bibtexKeyPatternTable.patternListProperty().bindBidirectional(viewModel.patternListProperty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class BibtexKeyPatternTabViewModel implements PreferenceTabViewModel {
private BooleanProperty letterAlwaysAddProperty = new SimpleBooleanProperty();
private StringProperty keyPatternRegexProperty = new SimpleStringProperty();
private StringProperty keyPatternReplacementProperty = new SimpleStringProperty();
private 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.
Expand Down Expand Up @@ -66,6 +67,7 @@ public void setValues() {

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));
}

@Override
Expand All @@ -92,6 +94,7 @@ public void storeSettings() {

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));
patternListProperty.forEach(item -> {
Expand Down Expand Up @@ -136,4 +139,6 @@ public void storeSettings() {
public ListProperty<BibtexKeyPatternPanelItemModel> patternListProperty() { return patternListProperty; }

public ObjectProperty<BibtexKeyPatternPanelItemModel> defaultKeyPatternProperty() { return defaultKeyPatternProperty; }

public StringProperty unwantedCharactersProperty() { return unwantedCharactersProperty; }
}
1 change: 0 additions & 1 deletion src/main/java/org/jabref/gui/preferences/GeneralTab.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
<CheckBox fx:id="inspectionWarningDuplicate"
text="%Warn about unresolved duplicates when closing inspection window"/>
<CheckBox fx:id="confirmDelete" text="%Show confirmation dialog when deleting entries"/>
<CheckBox fx:id="enforceLegalKeys" text="%Enforce legal characters in BibTeX keys"/>
<CheckBox fx:id="allowIntegerEdition" text="%Allow integers in 'edition' field in BibTeX mode"/>
<CheckBox fx:id="memoryStickMode"
text="%Load and Save preferences from/to jabref.xml on start-up (memory stick mode)"/>
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/org/jabref/gui/preferences/GeneralTabView.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public class GeneralTabView extends AbstractPreferenceTabView<GeneralTabViewMode
@FXML private ComboBox<BibDatabaseMode> biblatexMode;
@FXML private CheckBox inspectionWarningDuplicate;
@FXML private CheckBox confirmDelete;
@FXML private CheckBox enforceLegalKeys;
@FXML private CheckBox allowIntegerEdition;
@FXML private CheckBox memoryStickMode;
@FXML private CheckBox collectTelemetry;
Expand Down Expand Up @@ -86,7 +85,6 @@ public void initialize() {

inspectionWarningDuplicate.selectedProperty().bindBidirectional(viewModel.inspectionWarningDuplicateProperty());
confirmDelete.selectedProperty().bindBidirectional(viewModel.confirmDeleteProperty());
enforceLegalKeys.selectedProperty().bindBidirectional(viewModel.enforceLegalKeysProperty());
allowIntegerEdition.selectedProperty().bindBidirectional(viewModel.allowIntegerEditionProperty());
memoryStickMode.selectedProperty().bindBidirectional(viewModel.memoryStickModeProperty());
collectTelemetry.selectedProperty().bindBidirectional(viewModel.collectTelemetryProperty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public class GeneralTabViewModel implements PreferenceTabViewModel {
private final BooleanProperty confirmDeleteProperty = new SimpleBooleanProperty();
private final BooleanProperty memoryStickModeProperty = new SimpleBooleanProperty();
private final BooleanProperty collectTelemetryProperty = new SimpleBooleanProperty();
private final BooleanProperty enforceLegalKeysProperty = new SimpleBooleanProperty();
private final BooleanProperty allowIntegerEditionProperty = new SimpleBooleanProperty();
private final BooleanProperty showAdvancedHintsProperty = new SimpleBooleanProperty();
private final BooleanProperty markOwnerProperty = new SimpleBooleanProperty();
Expand Down Expand Up @@ -105,7 +104,6 @@ public void setValues() {

inspectionWarningDuplicateProperty.setValue(initialGeneralPreferences.isWarnAboutDuplicatesInInspection());
confirmDeleteProperty.setValue(initialGeneralPreferences.isConfirmDelete());
enforceLegalKeysProperty.setValue(initialGeneralPreferences.isEnforceLegalBibtexKey());
allowIntegerEditionProperty.setValue(initialGeneralPreferences.isAllowIntegerEditionBibtex());
memoryStickModeProperty.setValue(initialGeneralPreferences.isMemoryStickMode());
collectTelemetryProperty.setValue(preferencesService.shouldCollectTelemetry());
Expand Down Expand Up @@ -141,7 +139,6 @@ public void storeSettings() {
selectedBiblatexModeProperty.getValue(),
inspectionWarningDuplicateProperty.getValue(),
confirmDeleteProperty.getValue(),
enforceLegalKeysProperty.getValue(),
allowIntegerEditionProperty.getValue(),
memoryStickModeProperty.getValue(),
collectTelemetryProperty.getValue(),
Expand Down Expand Up @@ -199,8 +196,6 @@ public boolean validateSettings() {

public BooleanProperty collectTelemetryProperty() { return this.collectTelemetryProperty; }

public BooleanProperty enforceLegalKeysProperty() { return this.enforceLegalKeysProperty; }

public BooleanProperty allowIntegerEditionProperty() { return this.allowIntegerEditionProperty; }

public BooleanProperty showAdvancedHintsProperty() { return this.showAdvancedHintsProperty; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.jabref.logic.bibtexkeypattern;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
Expand All @@ -26,9 +27,10 @@ public class BibtexKeyGenerator extends BracketedPattern {
* All single characters that we can use for extending a key to make it unique.
*/
public static final String APPENDIX_CHARACTERS = "abcdefghijklmnopqrstuvwxyz";
public static final String DEFAULT_UNWANTED_CHARACTERS = "-`ʹ:!;?^+";
private static final Logger LOGGER = LoggerFactory.getLogger(BibtexKeyGenerator.class);
private static final String KEY_ILLEGAL_CHARACTERS = "{}(),\\\"-#~^:'`ʹ";
private static final String KEY_UNWANTED_CHARACTERS = "{}(),\\\"-";
// Source of disallowed characters : https://tex.stackexchange.com/a/408548/9075
private static final List<Character> DISALLOWED_CHARACTERS = Arrays.asList('{', '}', '(', ')', ',', '=', '\\', '"', '#', '%', '~', '\'');
private final AbstractBibtexKeyPattern citeKeyPattern;
private final BibDatabase database;
private final BibtexKeyPatternPreferences bibtexKeyPatternPreferences;
Expand All @@ -45,14 +47,16 @@ public BibtexKeyGenerator(AbstractBibtexKeyPattern citeKeyPattern, BibDatabase d
this.bibtexKeyPatternPreferences = Objects.requireNonNull(bibtexKeyPatternPreferences);
}

@Deprecated
static String generateKey(BibEntry entry, String pattern) {
return generateKey(entry, pattern, new BibDatabase());
}

@Deprecated
static String generateKey(BibEntry entry, String pattern, BibDatabase database) {
GlobalBibtexKeyPattern keyPattern = new GlobalBibtexKeyPattern(Collections.emptyList());
keyPattern.setDefaultValue("[" + pattern + "]");
return new BibtexKeyGenerator(keyPattern, database, new BibtexKeyPatternPreferences("", "", false, true, true, keyPattern, ',', false))
return new BibtexKeyGenerator(keyPattern, database, new BibtexKeyPatternPreferences("", "", false, true, keyPattern, ',', false, DEFAULT_UNWANTED_CHARACTERS))
.generateKey(entry);
}

Expand All @@ -73,36 +77,24 @@ private static String getAppendix(int number) {
}
}

public static String removeUnwantedCharacters(String key, boolean enforceLegalKey) {
if (!enforceLegalKey) {
// User doesn't want us to enforce legal characters. We must still look
// for whitespace and some characters such as commas, since these would
// interfere with parsing:
StringBuilder newKey = new StringBuilder();
for (int i = 0; i < key.length(); i++) {
char c = key.charAt(i);
if (KEY_UNWANTED_CHARACTERS.indexOf(c) == -1) {
newKey.append(c);
}
}
return newKey.toString();
}

StringBuilder newKey = new StringBuilder();
for (int i = 0; i < key.length(); i++) {
char c = key.charAt(i);
if (KEY_ILLEGAL_CHARACTERS.indexOf(c) == -1) {
newKey.append(c);
}
public static String removeUnwantedCharacters(String key, String unwantedCharacters) {
List<Character> unwantedChars = new ArrayList<>();
for (char ch : unwantedCharacters.toCharArray()) {
Copy link
Member

Choose a reason for hiding this comment

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

We don't need the unwantedChars variable, do we? Can you please delete that for loop and the variable? --> the code on line 86 directly accesses the method parameter unwantedChararacters!

Side note: This is an example why similar variable names can be mixed up.

Copy link
Contributor Author

@dextep dextep May 6, 2020

Choose a reason for hiding this comment

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

Of course, we don't need it, I'm sorry. Every time I get the impression that I didn't notice something. 🐞

unwantedChars.add(ch);
}

String newKey = key.chars()
.filter(c -> unwantedCharacters.indexOf(c) == -1)
.filter(c -> !DISALLOWED_CHARACTERS.contains((char) c))
.collect(StringBuilder::new,
StringBuilder::appendCodePoint, StringBuilder::append)
.toString();
// Replace non-English characters like umlauts etc. with a sensible
// letter or letter combination that bibtex can accept.
return StringUtil.replaceSpecialCharacters(newKey.toString());
return StringUtil.replaceSpecialCharacters(newKey);
}

public static String cleanKey(String key, boolean enforceLegalKey) {
return removeUnwantedCharacters(key, enforceLegalKey).replaceAll("\\s", "");
public static String cleanKey(String key, String unwantedCharacters) {
return removeUnwantedCharacters(key, unwantedCharacters).replaceAll("\\s", "");
}

public String generateKey(BibEntry entry) {
Expand All @@ -128,17 +120,14 @@ public String generateKey(BibEntry entry) {
List<String> parts = parseFieldMarker(typeListEntry);
Character delimiter = bibtexKeyPatternPreferences.getKeywordDelimiter();
String pattern = "[" + parts.get(0) + "]";
String label = expandBrackets(pattern, delimiter, entry, database, bibtexKeyPatternPreferences.isEnforceLegalKey());
String label = expandBrackets(pattern, delimiter, entry, database);
// apply modifier if present
if (parts.size() > 1) {
label = applyModifiers(label, parts, 1);
}

// Remove all illegal characters from the label.
label = cleanKey(label, bibtexKeyPatternPreferences.isEnforceLegalKey());

label = cleanKey(label, bibtexKeyPatternPreferences.getUnwantedCharacters());
stringBuilder.append(label);

} else {
stringBuilder.append(typeListEntry);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,28 @@ public class BibtexKeyPatternPreferences {
private final String keyPatternReplacement;
private final boolean alwaysAddLetter;
private final boolean firstLetterA;
private final boolean enforceLegalKey;
private final GlobalBibtexKeyPattern keyPattern;
private Character keywordDelimiter;
private boolean avoidOverwritingCiteKey;
private String unwantedCharacters;

public BibtexKeyPatternPreferences(String keyPatternRegex,
String keyPatternReplacement,
boolean alwaysAddLetter,
boolean firstLetterA,
boolean enforceLegalKey,
GlobalBibtexKeyPattern keyPattern,
Character keywordDelimiter,
boolean avoidOverwritingCiteKey) {
boolean avoidOverwritingCiteKey,
String unwantedCharacters) {

this.keyPatternRegex = keyPatternRegex;
this.keyPatternReplacement = keyPatternReplacement;
this.alwaysAddLetter = alwaysAddLetter;
this.firstLetterA = firstLetterA;
this.enforceLegalKey = enforceLegalKey;
this.keyPattern = keyPattern;
this.keywordDelimiter = keywordDelimiter;
this.avoidOverwritingCiteKey = avoidOverwritingCiteKey;
this.unwantedCharacters = unwantedCharacters;
}

public String getKeyPatternRegex() {
Expand All @@ -48,8 +48,8 @@ public boolean isFirstLetterA() {
return firstLetterA;
}

public boolean isEnforceLegalKey() {
return enforceLegalKey;
public String getUnwantedCharacters() {
return unwantedCharacters;
}

public GlobalBibtexKeyPattern getKeyPattern() {
Expand Down
Loading