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

Fix package of PreviewLayout #5702

Merged
merged 21 commits into from
Mar 29, 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
22 changes: 22 additions & 0 deletions docs/adr/0008-use-public-final-instead-of-getters.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Use `public final` instead of getters to offer access to immutable variables

## Context and Problem Statement

When making immutable data accessible in a java class, should it be using getters or by non-modifiable fields?

## Considered Options

* Offer public static field
* Offer getters

## Decision Outcome

Chosen option: "Offer public static field", because getters used to be a convention which was even more manifested due to libraries depending on the existence on getters/setters. In the case of immutable variables, adding public getters is just useless since one is not hiding anything.

### Positive Consequences

* Shorter code

### Negative Consequences

* new comers could get confused, because getters/setters are still teached
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/DragAndDropDataFormats.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import javafx.scene.input.DataFormat;

import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.preview.PreviewLayout;

/**
* Contains all the different {@link DataFormat}s that may occur in JabRef.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/maintable/RightClickMenu.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import org.jabref.gui.specialfields.SpecialFieldMenuItemFactory;
import org.jabref.logic.citationstyle.CitationStyleOutputFormat;
import org.jabref.logic.citationstyle.CitationStylePreviewLayout;
import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.model.entry.field.SpecialField;
import org.jabref.preferences.JabRefPreferences;
import org.jabref.preferences.PreferencesService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.ValueTableCellFactory;
import org.jabref.gui.util.ViewModelTableRowFactory;
import org.jabref.logic.citationstyle.TextBasedPreviewLayout;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.layout.TextBasedPreviewLayout;
import org.jabref.logic.openoffice.OOBibStyle;
import org.jabref.logic.openoffice.StyleLoader;
import org.jabref.logic.util.TestEntry;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import org.jabref.gui.preview.PreviewViewer;
import org.jabref.gui.util.IconValidationDecorator;
import org.jabref.gui.util.ViewModelListCellFactory;
import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.logic.util.TestEntry;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.preferences.JabRefPreferences;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
import org.jabref.gui.util.TaskExecutor;
import org.jabref.logic.citationstyle.CitationStyle;
import org.jabref.logic.citationstyle.CitationStylePreviewLayout;
import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.citationstyle.TextBasedPreviewLayout;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.layout.TextBasedPreviewLayout;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.preferences.JabRefPreferences;
import org.jabref.preferences.PreviewPreferences;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
import org.jabref.logic.citationstyle.CitationStyleGenerator;
import org.jabref.logic.citationstyle.CitationStyleOutputFormat;
import org.jabref.logic.citationstyle.CitationStylePreviewLayout;
import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.layout.Layout;
import org.jabref.logic.layout.LayoutFormatterPreferences;
import org.jabref.logic.layout.LayoutHelper;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.logic.util.OS;
import org.jabref.model.entry.BibEntry;
import org.jabref.preferences.PreviewPreferences;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/preview/PreviewPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.keyboard.KeyBinding;
import org.jabref.gui.keyboard.KeyBindingRepository;
import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.preferences.JabRefPreferences;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/preview/PreviewViewer.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.gui.util.ThemeLoader;
import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.exporter.ExporterFactory;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.logic.search.SearchQuery;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import org.jabref.gui.DragAndDropDataFormats;
import org.jabref.gui.StateManager;
import org.jabref.logic.citationstyle.PreviewLayout;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.model.entry.BibEntry;

/**
Expand Down
6 changes: 1 addition & 5 deletions src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,7 @@ public void writeWithoutPrependedNewlines(BibEntry entry, Writer out, BibDatabas
}

/**
* Write fields in the order of requiredFields, optionalFields and other fields, but does not sort the fields.
*
* @param entry
* @param out
* @throws IOException
* Writes fields in the order of requiredFields, optionalFields and other fields, but does not sort the fields.
*/
private void writeRequiredFieldsFirstRemainingFieldsSecond(BibEntry entry, Writer out,
BibDatabaseMode bibDatabaseMode) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ public FieldContentFormatter(FieldContentFormatterPreferences preferences) {
*/
public String format(String fieldContent, Field field) {
if (multiLineFields.contains(field)) {
// Unify line breaks
return StringUtil.unifyLineBreaks(fieldContent, OS.NEWLINE);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ public class FieldContentFormatterPreferences {

private final List<Field> nonWrappableFields;

/**
* Constructor defining that there are not any non-wrappable fields
*/
public FieldContentFormatterPreferences() {
// This constructor is only to allow an empty constructor in SavePreferences
this.nonWrappableFields = Collections.emptyList();
}

Expand Down
56 changes: 35 additions & 21 deletions src/main/java/org/jabref/logic/bibtex/FieldWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,22 @@
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.strings.StringUtil;

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

/**
* Obeys following settings:
* * JabRefPreferences.RESOLVE_STRINGS_ALL_FIELDS
* * JabRefPreferences.DO_NOT_RESOLVE_STRINGS_FOR
* * JabRefPreferences.WRITEFIELD_WRAPFIELD
* Converts JabRef's internal BibTeX representation of a BibTeX field to BibTeX text representation
*/
public class FieldWriter {

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

private static final char FIELD_START = '{';
private static final char FIELD_END = '}';

private final boolean neverFailOnHashes;
private final FieldWriterPreferences preferences;
private final FieldContentFormatter formatter;
private StringBuilder stringBuilder;

public FieldWriter(FieldWriterPreferences preferences) {
this(true, preferences);
Expand Down Expand Up @@ -58,12 +60,15 @@ private static void checkBraces(String text) throws InvalidFieldValueException {

// Then we throw an exception if the error criteria are met.
if (!(right == 0) && (left == 0)) {
LOGGER.error("Unescaped '}' character without opening bracket ends string prematurely. Field value: {}", text );
throw new InvalidFieldValueException("Unescaped '}' character without opening bracket ends string prematurely. Field value: " + text);
}
if (!(right == 0) && (right < left)) {
LOGGER.error("Unescaped '}' character without opening bracket ends string prematurely. Field value: {}", text);
throw new InvalidFieldValueException("Unescaped '}' character without opening bracket ends string prematurely. Field value: " + text);
}
if (left != right) {
LOGGER.error("Braces don't match. Field value: {}", text);
throw new InvalidFieldValueException("Braces don't match. Field value: " + text);
}
}
Expand Down Expand Up @@ -95,9 +100,10 @@ public String write(Field field, String content) throws InvalidFieldValueExcepti
* For instance, <code>#jan# - #feb#</code> gets <code>jan #{ - } # feb</code> (see @link{org.jabref.logic.bibtex.LatexFieldFormatterTests#makeHashEnclosedWordsRealStringsInMonthField()})
*/
private String formatAndResolveStrings(String content, Field field) throws InvalidFieldValueException {
stringBuilder = new StringBuilder();
checkBraces(content);

StringBuilder stringBuilder = new StringBuilder();

// Here we assume that the user encloses any bibtex strings in #, e.g.:
// #jan# - #feb#
// ...which will be written to the file like this:
Expand Down Expand Up @@ -126,6 +132,9 @@ private String formatAndResolveStrings(String content, Field field) throws Inval
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 '\\#'. "
+ "In JabRef, use pairs of # characters to indicate a string. "
+ "Note that the entry causing the problem has been selected. Field value: {}", content);
throw new InvalidFieldValueException(
"The # character is not allowed in BibTeX strings unless escaped as in '\\#'.\n"
+ "In JabRef, use pairs of # characters to indicate a string.\n"
Expand All @@ -135,13 +144,13 @@ private String formatAndResolveStrings(String content, Field field) throws Inval
}

if (pos1 > pivot) {
writeText(content, pivot, pos1);
writeText(stringBuilder, content, pivot, pos1);
}
if ((pos1 < content.length()) && ((pos2 - 1) > pos1)) {
// We check that the string label is not empty. That means
// an occurrence of ## will simply be ignored. Should it instead
// cause an error message?
writeStringLabel(content, pos1 + 1, pos2, pos1 == pivot,
writeStringLabel(stringBuilder, content, pos1 + 1, pos2, pos1 == pivot,
(pos2 + 1) == content.length());
}

Expand All @@ -168,28 +177,33 @@ private boolean shouldResolveStrings(Field field) {
private String formatWithoutResolvingStrings(String content, Field field) throws InvalidFieldValueException {
checkBraces(content);

stringBuilder = new StringBuilder(String.valueOf(FIELD_START));

StringBuilder stringBuilder = new StringBuilder(String.valueOf(FIELD_START));
stringBuilder.append(formatter.format(content, field));

stringBuilder.append(FIELD_END);

return stringBuilder.toString();
}

private void writeText(String text, int startPos, int endPos) {
/**
* @param stringBuilder the StringBuilder to append the text to
* @param text the text to append
*/
private void writeText(StringBuilder stringBuilder, String text, int startPos, int endPos) {
stringBuilder.append(FIELD_START);
stringBuilder.append(text, startPos, endPos);
stringBuilder.append(FIELD_END);
}

private void writeStringLabel(String text, int startPos, int endPos, boolean first, boolean last) {
putIn((first ? "" : " # ") + text.substring(startPos, endPos)
+ (last ? "" : " # "));
}

private void putIn(String s) {
stringBuilder.append(StringUtil.wrap(s, preferences.getLineLength(), OS.NEWLINE));
/**
* @param stringBuilder the StringBuilder to append the text to
* @param text the text use as basis to get the text to append
* @param startPos the position in text where the text to add starts
* @param endPos the position in text where the text to add ends
* @param isFirst true if the label to write is the first one to write
* @param isLast true if the label to write is the last one to write
*/
private void writeStringLabel(StringBuilder stringBuilder, String text, int startPos, int endPos, boolean isFirst, boolean isLast) {
String line = (isFirst ? "" : " # ") + text.substring(startPos, endPos) + (isLast ? "" : " # ");
String wrappedLine = StringUtil.wrap(line, preferences.getLineLength(), OS.NEWLINE);
stringBuilder.append(wrappedLine);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ public FieldWriterPreferences(boolean resolveStringsAllFields, List<Field> doNot
this.fieldContentFormatterPreferences = fieldContentFormatterPreferences;
}

/**
* 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.
*/
public FieldWriterPreferences() {
// This constructor is only to allow an empty constructor in SavePreferences
this(true, Collections.emptyList(), new FieldContentFormatterPreferences());
Expand Down
83 changes: 83 additions & 0 deletions src/main/java/org/jabref/logic/bst/BstPreviewLayout.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package org.jabref.logic.bst;
Copy link
Member

Choose a reason for hiding this comment

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

I would move this also to the citationstyle package, because it's not really about bst (which is only used as a tool)

Copy link
Member Author

Choose a reason for hiding this comment

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

I see org.jabref.logic.preview as the other possibility to collect the preview generators. -- I would keep the package org.jabref.logic.citationstyle for CSL only and keep out the relation to .bst and to our other Layouts.


import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;

import org.jabref.logic.cleanup.ConvertToBibtexCleanup;
import org.jabref.logic.formatter.bibtexfields.RemoveNewlinesFormatter;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.layout.format.LatexToUnicodeFormatter;
import org.jabref.logic.layout.format.RemoveLatexCommandsFormatter;
import org.jabref.logic.layout.format.RemoveTilde;
import org.jabref.logic.preview.PreviewLayout;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.BibEntry;

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

public class BstPreviewLayout implements PreviewLayout {

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

private final String name;

private VM vm;
private String error;

public BstPreviewLayout(Path path) {
name = path.getFileName().toString();
if (!Files.exists(path)) {
LOGGER.error("File {} not found", path.toAbsolutePath());
error = Localization.lang("Error opening file '%0'.", path.toString());
return;
}
try {
vm = new VM(path.toFile());
} catch (Exception e) {
LOGGER.error("Could not read {}.", path.toAbsolutePath(), e);
error = Localization.lang("Error opening file '%0'.", path.toString());
}
}

@Override
public String generatePreview(BibEntry originalEntry, BibDatabase database) {
if (error != null) {
return error;
}
// ensure that the entry is of BibTeX format (and do not modify the original entry)
BibEntry entry = (BibEntry) originalEntry.clone();
new ConvertToBibtexCleanup().cleanup(entry);
String result = vm.run(List.of(entry));
// Remove all comments
result = result.replaceAll("%.*", "");
// Remove all LaTeX comments
// The RemoveLatexCommandsFormatter keeps the words inside latex environments. Therefore, we remove them manually
result = result.replace("\\begin{thebibliography}{1}", "");
result = result.replace("\\end{thebibliography}", "");
// The RemoveLatexCommandsFormatter keeps the word inside the latex command, but we want to remove that completely
result = result.replaceAll("\\\\bibitem[{].*[}]", "");
// We want to replace \newblock by a space instead of completely removing it
result = result.replace("\\newblock", " ");
// remove all latex commands statements - assumption: command in a separate line
result = result.replaceAll("(?m)^\\\\.*$", "");
// remove some IEEEtran.bst output (resulting from a multiline \providecommand)
result = result.replace("#2}}", "");
// Have quotes right - and more
result = new LatexToUnicodeFormatter().format(result);
result = result.replace("``", "\"");
result = result.replace("''", "\"");
// Final cleanup
result = new RemoveNewlinesFormatter().format(result);
result = new RemoveLatexCommandsFormatter().format(result);
result = new RemoveTilde().format(result);
result = result.trim().replaceAll(" +", " ");
return result;
}

@Override
public String getName() {
return name;
}
}
Loading