Skip to content

Commit f3b8912

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #1125 from HubSpot/revert-whitespace-changes
Revert whitespace changes
2 parents 3b99a0a + ca98de9 commit f3b8912

9 files changed

Lines changed: 38 additions & 235 deletions

File tree

‎src/main/java/com/hubspot/jinjava/LegacyOverrides.java‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,16 @@
33
/**
44
* This class allows Jinjava to be configured to override legacy behaviour.
55
* LegacyOverrides.NONE signifies that none of the legacy functionality will be overridden.
6-
* LegacyOverrides.ALL signifies that all new functionality will be used; avoid legacy "bugs".
76
*/
87
public class LegacyOverrides {
98
public static final LegacyOverrides NONE = new LegacyOverrides.Builder().build();
10-
public static final LegacyOverrides ALL = new LegacyOverrides.Builder()
11-
.withEvaluateMapKeys(true)
12-
.withIterateOverMapKeys(true)
13-
.withUsePyishObjectMapper(true)
14-
.withUseSnakeCasePropertyNaming(true)
15-
.withWhitespaceRequiredWithinTokens(true)
16-
.withUseNaturalOperatorPrecedence(true)
17-
.withParseWhitespaceControlStrictly(true)
18-
.withAllowAdjacentTextNodes(true)
19-
.build();
209
private final boolean evaluateMapKeys;
2110
private final boolean iterateOverMapKeys;
2211
private final boolean usePyishObjectMapper;
2312
private final boolean useSnakeCasePropertyNaming;
2413
private final boolean whitespaceRequiredWithinTokens;
2514
private final boolean useNaturalOperatorPrecedence;
2615
private final boolean parseWhitespaceControlStrictly;
27-
private final boolean allowAdjacentTextNodes;
2816

2917
private LegacyOverrides(Builder builder) {
3018
evaluateMapKeys = builder.evaluateMapKeys;
@@ -34,7 +22,6 @@ private LegacyOverrides(Builder builder) {
3422
whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens;
3523
useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence;
3624
parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly;
37-
allowAdjacentTextNodes = builder.allowAdjacentTextNodes;
3825
}
3926

4027
public static Builder newBuilder() {
@@ -69,10 +56,6 @@ public boolean isParseWhitespaceControlStrictly() {
6956
return parseWhitespaceControlStrictly;
7057
}
7158

72-
public boolean isAllowAdjacentTextNodes() {
73-
return allowAdjacentTextNodes;
74-
}
75-
7659
public static class Builder {
7760
private boolean evaluateMapKeys = false;
7861
private boolean iterateOverMapKeys = false;
@@ -81,7 +64,6 @@ public static class Builder {
8164
private boolean whitespaceRequiredWithinTokens = false;
8265
private boolean useNaturalOperatorPrecedence = false;
8366
private boolean parseWhitespaceControlStrictly = false;
84-
private boolean allowAdjacentTextNodes = false;
8567

8668
private Builder() {}
8769

@@ -101,8 +83,7 @@ public static Builder from(LegacyOverrides legacyOverrides) {
10183
.withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence)
10284
.withParseWhitespaceControlStrictly(
10385
legacyOverrides.parseWhitespaceControlStrictly
104-
)
105-
.withAllowAdjacentTextNodes(legacyOverrides.allowAdjacentTextNodes);
86+
);
10687
}
10788

10889
public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
@@ -145,10 +126,5 @@ public Builder withParseWhitespaceControlStrictly(
145126
this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly;
146127
return this;
147128
}
148-
149-
public Builder withAllowAdjacentTextNodes(boolean allowAdjacentTextNodes) {
150-
this.allowAdjacentTextNodes = allowAdjacentTextNodes;
151-
return this;
152-
}
153129
}
154130
}

‎src/main/java/com/hubspot/jinjava/tree/TreeParser.java‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,9 @@ public Node buildTree() {
6262
Node node = nextNode();
6363

6464
if (node != null) {
65-
if (
66-
node instanceof TextNode &&
67-
getLastSibling() instanceof TextNode &&
68-
!interpreter.getConfig().getLegacyOverrides().isAllowAdjacentTextNodes()
69-
) {
65+
if (node instanceof TextNode && getLastSibling() instanceof TextNode) {
7066
// merge adjacent text nodes so whitespace control properly applies
71-
((TextToken) getLastSibling().getMaster()).mergeImageAndContent(
72-
(TextToken) node.getMaster()
73-
);
67+
getLastSibling().getMaster().mergeImageAndContent(node.getMaster());
7468
} else {
7569
parent.getChildren().add(node);
7670
}
@@ -102,12 +96,6 @@ public Node buildTree() {
10296

10397
private Node nextNode() {
10498
Token token = scanner.next();
105-
if (token.isLeftTrim()) {
106-
final Node lastSibling = getLastSibling();
107-
if (lastSibling instanceof TextNode) {
108-
lastSibling.getMaster().setRightTrim(true);
109-
}
110-
}
11199

112100
if (token.getType() == symbols.getFixed()) {
113101
if (token instanceof UnclosedToken) {
@@ -182,7 +170,7 @@ private Node text(TextToken textToken) {
182170
final Node lastSibling = getLastSibling();
183171

184172
// if last sibling was a tag and has rightTrimAfterEnd, strip whitespace
185-
if (lastSibling != null && isRightTrim(lastSibling)) {
173+
if (lastSibling instanceof TagNode && isRightTrim((TagNode) lastSibling)) {
186174
textToken.setLeftTrim(true);
187175
}
188176

@@ -198,21 +186,18 @@ private Node text(TextToken textToken) {
198186
return n;
199187
}
200188

201-
private boolean isRightTrim(Node lastSibling) {
202-
if (lastSibling instanceof TagNode) {
203-
return (
204-
((TagNode) lastSibling).getEndName() == null ||
205-
(
206-
((TagNode) lastSibling).getTag() instanceof FlexibleTag &&
207-
!((FlexibleTag) ((TagNode) lastSibling).getTag()).hasEndTag(
208-
(TagToken) lastSibling.getMaster()
209-
)
210-
)
189+
private boolean isRightTrim(TagNode lastSibling) {
190+
return (
191+
lastSibling.getEndName() == null ||
192+
(
193+
lastSibling.getTag() instanceof FlexibleTag &&
194+
!((FlexibleTag) lastSibling.getTag()).hasEndTag(
195+
(TagToken) lastSibling.getMaster()
196+
)
211197
)
212-
? lastSibling.getMaster().isRightTrim()
213-
: lastSibling.getMaster().isRightTrimAfterEnd();
214-
}
215-
return lastSibling.getMaster().isRightTrim();
198+
)
199+
? lastSibling.getMaster().isRightTrim()
200+
: lastSibling.getMaster().isRightTrimAfterEnd();
216201
}
217202

218203
private Node expression(ExpressionToken expressionToken) {
@@ -257,6 +242,14 @@ private Node tag(TagToken tagToken) {
257242
if (tag instanceof EndTag) {
258243
endTag(tag, tagToken);
259244
return null;
245+
} else {
246+
// if a tag has left trim, mark the last sibling to trim right whitespace
247+
if (tagToken.isLeftTrim()) {
248+
final Node lastSibling = getLastSibling();
249+
if (lastSibling instanceof TextNode) {
250+
lastSibling.getMaster().setRightTrim(true);
251+
}
252+
}
260253
}
261254

262255
TagNode node = new TagNode(tag, tagToken, symbols);
@@ -275,6 +268,16 @@ private Node tag(TagToken tagToken) {
275268
}
276269

277270
private void endTag(Tag tag, TagToken tagToken) {
271+
final Node lastSibling = getLastSibling();
272+
273+
if (
274+
parent instanceof TagNode &&
275+
tagToken.isLeftTrim() &&
276+
lastSibling instanceof TextNode
277+
) {
278+
lastSibling.getMaster().setRightTrim(true);
279+
}
280+
278281
if (parent.getMaster() != null) { // root node
279282
parent.getMaster().setRightTrimAfterEnd(tagToken.isRightTrim());
280283
}

‎src/main/java/com/hubspot/jinjava/tree/output/OutputList.java‎

Lines changed: 0 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,11 @@
33
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
44
import com.hubspot.jinjava.interpret.OutputTooBigException;
55
import com.hubspot.jinjava.interpret.TemplateError;
6-
import com.hubspot.jinjava.tree.parse.TokenScannerSymbols;
76
import com.hubspot.jinjava.util.LengthLimitingStringBuilder;
87
import java.util.LinkedList;
98
import java.util.List;
109

1110
public class OutputList {
12-
public static final String PREVENT_ACCIDENTAL_EXPRESSIONS =
13-
"PREVENT_ACCIDENTAL_EXPRESSIONS";
1411
private final List<OutputNode> nodes = new LinkedList<>();
1512
private final List<BlockPlaceholderOutputNode> blocks = new LinkedList<>();
1613
private final long maxOutputSize;
@@ -51,72 +48,6 @@ public List<BlockPlaceholderOutputNode> getBlocks() {
5148
public String getValue() {
5249
LengthLimitingStringBuilder val = new LengthLimitingStringBuilder(maxOutputSize);
5350

54-
return JinjavaInterpreter
55-
.getCurrentMaybe()
56-
.map(JinjavaInterpreter::getConfig)
57-
.filter(
58-
config ->
59-
config
60-
.getFeatures()
61-
.getActivationStrategy(PREVENT_ACCIDENTAL_EXPRESSIONS)
62-
.isActive(null)
63-
)
64-
.map(
65-
config -> joinNodesWithoutAddingExpressions(val, config.getTokenScannerSymbols())
66-
)
67-
.orElseGet(() -> joinNodes(val));
68-
}
69-
70-
private String joinNodesWithoutAddingExpressions(
71-
LengthLimitingStringBuilder val,
72-
TokenScannerSymbols tokenScannerSymbols
73-
) {
74-
String separator = getWhitespaceSeparator(tokenScannerSymbols);
75-
String prev = null;
76-
String cur;
77-
for (OutputNode node : nodes) {
78-
try {
79-
cur = node.getValue();
80-
if (
81-
prev != null &&
82-
prev.length() > 0 &&
83-
prev.charAt(prev.length() - 1) == tokenScannerSymbols.getExprStartChar()
84-
) {
85-
if (
86-
cur.length() > 0 &&
87-
TokenScannerSymbols.isNoteTagOrExprChar(tokenScannerSymbols, cur.charAt(0))
88-
) {
89-
val.append(separator);
90-
}
91-
}
92-
prev = cur;
93-
val.append(node.getValue());
94-
} catch (OutputTooBigException e) {
95-
JinjavaInterpreter
96-
.getCurrent()
97-
.addError(TemplateError.fromOutputTooBigException(e));
98-
return val.toString();
99-
}
100-
}
101-
102-
return val.toString();
103-
}
104-
105-
private static String getWhitespaceSeparator(TokenScannerSymbols tokenScannerSymbols) {
106-
@SuppressWarnings("StringBufferReplaceableByString")
107-
String separator = new StringBuilder()
108-
.append('\n')
109-
.append(tokenScannerSymbols.getPrefixChar())
110-
.append(tokenScannerSymbols.getNoteChar())
111-
.append(tokenScannerSymbols.getTrimChar())
112-
.append(' ')
113-
.append(tokenScannerSymbols.getNoteChar())
114-
.append(tokenScannerSymbols.getExprEndChar())
115-
.toString();
116-
return separator;
117-
}
118-
119-
private String joinNodes(LengthLimitingStringBuilder val) {
12051
for (OutputNode node : nodes) {
12152
try {
12253
val.append(node.getValue());

‎src/main/java/com/hubspot/jinjava/tree/parse/NoteToken.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
**********************************************************************/
1616
package com.hubspot.jinjava.tree.parse;
1717

18-
import org.apache.commons.lang3.StringUtils;
19-
2018
public class NoteToken extends Token {
2119
private static final long serialVersionUID = -3859011447900311329L;
2220

@@ -39,9 +37,6 @@ public int getType() {
3937
*/
4038
@Override
4139
protected void parse() {
42-
if (StringUtils.isNotEmpty(image)) {
43-
handleTrim(image.substring(2, image.length() - 2));
44-
}
4540
content = "";
4641
}
4742

‎src/main/java/com/hubspot/jinjava/tree/parse/TextToken.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,6 @@ public TextToken(
2929
super(image, lineNumber, startPosition, symbols);
3030
}
3131

32-
public void mergeImageAndContent(TextToken otherToken) {
33-
String thisOutput = output();
34-
String otherTokenOutput = otherToken.output();
35-
this.image = thisOutput + otherTokenOutput;
36-
this.content = image;
37-
}
38-
3932
@Override
4033
public int getType() {
4134
return getSymbols().getFixed();

‎src/main/java/com/hubspot/jinjava/tree/parse/Token.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ public String getImage() {
5353
return image;
5454
}
5555

56+
public void mergeImageAndContent(Token otherToken) {
57+
this.image = image + otherToken.image;
58+
this.content = content + otherToken.content;
59+
}
60+
5661
public int getLineNumber() {
5762
return lineNumber;
5863
}

‎src/main/java/com/hubspot/jinjava/tree/parse/TokenScannerSymbols.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,4 @@ public String getClosingComment() {
122122
}
123123
return closingComment;
124124
}
125-
126-
public static boolean isNoteTagOrExprChar(TokenScannerSymbols symbols, char c) {
127-
return (
128-
c == symbols.getNote() || c == symbols.getTag() || c == symbols.getExprStartChar()
129-
);
130-
}
131125
}

‎src/test/java/com/hubspot/jinjava/interpret/JinjavaInterpreterTest.java‎

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,15 @@
88
import com.google.common.collect.Lists;
99
import com.hubspot.jinjava.Jinjava;
1010
import com.hubspot.jinjava.JinjavaConfig;
11-
import com.hubspot.jinjava.features.FeatureConfig;
12-
import com.hubspot.jinjava.features.FeatureStrategies;
1311
import com.hubspot.jinjava.interpret.JinjavaInterpreter.InterpreterScopeClosable;
1412
import com.hubspot.jinjava.interpret.TemplateError.ErrorItem;
1513
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
1614
import com.hubspot.jinjava.interpret.TemplateError.ErrorType;
17-
import com.hubspot.jinjava.mode.EagerExecutionMode;
1815
import com.hubspot.jinjava.mode.PreserveRawExecutionMode;
1916
import com.hubspot.jinjava.objects.date.FormattedDate;
2017
import com.hubspot.jinjava.objects.date.StrftimeFormatter;
2118
import com.hubspot.jinjava.tree.TextNode;
2219
import com.hubspot.jinjava.tree.output.BlockInfo;
23-
import com.hubspot.jinjava.tree.output.OutputList;
2420
import com.hubspot.jinjava.tree.parse.TextToken;
2521
import com.hubspot.jinjava.tree.parse.TokenScannerSymbols;
2622
import java.time.ZoneId;
@@ -507,53 +503,4 @@ public void itFiltersDuplicateErrors() {
507503

508504
assertThat(interpreter.getErrors()).containsExactly(error1, error2);
509505
}
510-
511-
@Test
512-
public void itPreventsAccidentalExpressions() {
513-
String makeExpression = "if (true) {\n{%- print deferred -%}\n}";
514-
String makeTag = "if (true) {\n{%- print '% print 123 %' -%}\n}";
515-
String makeNote = "if (true) {\n{%- print '# note #' -%}\n}";
516-
jinjava.getGlobalContext().put("deferred", DeferredValue.instance());
517-
518-
JinjavaInterpreter normalInterpreter = new JinjavaInterpreter(
519-
jinjava,
520-
jinjava.getGlobalContext(),
521-
JinjavaConfig.newBuilder().withExecutionMode(EagerExecutionMode.instance()).build()
522-
);
523-
JinjavaInterpreter preventingInterpreter = new JinjavaInterpreter(
524-
jinjava,
525-
jinjava.getGlobalContext(),
526-
JinjavaConfig
527-
.newBuilder()
528-
.withFeatureConfig(
529-
FeatureConfig
530-
.newBuilder()
531-
.add(OutputList.PREVENT_ACCIDENTAL_EXPRESSIONS, FeatureStrategies.ACTIVE)
532-
.build()
533-
)
534-
.withExecutionMode(EagerExecutionMode.instance())
535-
.build()
536-
);
537-
JinjavaInterpreter.pushCurrent(normalInterpreter);
538-
try {
539-
assertThat(normalInterpreter.render(makeExpression))
540-
.isEqualTo("if (true) {{% print deferred %}}");
541-
assertThat(normalInterpreter.render(makeTag))
542-
.isEqualTo("if (true) {% print 123 %}");
543-
assertThat(normalInterpreter.render(makeNote)).isEqualTo("if (true) {# note #}");
544-
} finally {
545-
JinjavaInterpreter.popCurrent();
546-
}
547-
JinjavaInterpreter.pushCurrent(preventingInterpreter);
548-
try {
549-
assertThat(preventingInterpreter.render(makeExpression))
550-
.isEqualTo("if (true) {\n" + "{#- #}{% print deferred %}}");
551-
assertThat(preventingInterpreter.render(makeTag))
552-
.isEqualTo("if (true) {\n" + "{#- #}% print 123 %}");
553-
assertThat(preventingInterpreter.render(makeNote))
554-
.isEqualTo("if (true) {\n" + "{#- #}# note #}");
555-
} finally {
556-
JinjavaInterpreter.popCurrent();
557-
}
558-
}
559506
}

0 commit comments

Comments
 (0)