PebbleTemplates
Whitespace Control with for loop and nested if statement
I am not able to control the vertical spacing/whitespace when I have a template that has an if statement inside a for loop that ends up skipping some of the items in the list. I have the Pebble Engine configured to disable New Line Trimming since I see no way to get the vertical spacing that I want with New Line Trimming enabled.
I have made a JUnit test to demonstrate the issue. The example template as well as expected template output and actual template output is shown below. I will submit a pull requests with the test code. One test is failing due to this issue. That test is marked with @Ignore so that adding the tests does not break the build.
Template:
text before for loop followed by blank line
{% for item in items %}
{% if item.itemType equals "ITEM_TYPE1" -%}
Item 1
{% elseif item.itemType equals "ITEM_TYPE2" -%}
Item 2
{% endif -%}
{% endfor %}
text after for loop preceded by blank line
Expected Template Output:
text before for loop followed by blank line
Item 1
Item 2
text after for loop preceded by blank line
Actual Template Output:
text before for loop followed by blank line
Item 1
Item 2
text after for loop preceded by blank line
This is a big problem for me since I am using Pebble for code generation. This results in generated code such as the following, which is a real problem for generated code. I will be glad to work on fixing this issue in the Pebble code if you confirm that this is an issue. If there is another way to get the desired output in a Pebble template, please let me know.
public class SchoolModel implements Serializable {
private static final long serialVersionUID = 1L;
private Long id;
public String identifier;
@NotNull
private String abbreviation;
private LocalTime goHomeNotesDailyCutoffTime;
private LocalDate goHomeNotesStartDate;
private LocalDate goHomeNotesStopDate;
@NotNull
private String name;
public String getIdentifier() {
return this.identifier;
}
public void setIdentifier(String identifier) {
this.identifier = identifier;
}
public Long getId() {
return id;
}
public void setId(Long id) {
this.id = id;
}
public String getAbbreviation() {
return this.abbreviation;
}
public void setAbbreviation(String abbreviation) {
this.abbreviation = abbreviation;
}
public LocalTime getGoHomeNotesDailyCutoffTime() {
return this.goHomeNotesDailyCutoffTime;
}
If you use this template, it's gonna work. I've modified the {%- endif -%} and {% endfor -%}
text before for loop followed by blank line
{% for item in items %}
{% if item.itemType equals "ITEM_TYPE1" -%}
Item 1
{% elseif item.itemType equals "ITEM_TYPE2" -%}
Item 2
{%- endif -%}
{% endfor -%}
text after for loop preceded by blank line
Cool! Yea, that works for the test case (I have not tested it with my real scenario yet). Can you explain why? I have many cases that I do this or something similar. I need to be able to think clearly about the whitespace behavior.
It seems that I have to specify the whitespace control differently depending on if I have one, two, or three "elseif" statements in order to get the same spacing in the output.
Template with one elseif statements:
text before for loop followed by blank line
{% for item in items %}
{% if item.itemType equals "ITEM_TYPE1" -%}
Item 1
{% elseif item.itemType equals "ITEM_TYPE2" -%}
Item 2
{%- endif -%}
{% endfor -%}
text after for loop preceded by blank line
Template with two elseif statements:
text before for loop followed by blank line
{% for item in items %}
{% if item.itemType equals "ITEM_TYPE1" -%}
Item 1
{% elseif item.itemType equals "ITEM_TYPE2" -%}
Item 2
{% elseif item.itemType equals "ITEM_TYPE3" -%}
Item 3
{%- endif -%}
{% endfor %}
text after for loop preceded by blank line
Template with 3 elseif statements:
text before for loop followed by blank line
{% for item in items %}
{% if item.itemType equals "ITEM_TYPE1" -%}
Item 1
{% elseif item.itemType equals "ITEM_TYPE2" -%}
Item 2
{% elseif item.itemType equals "ITEM_TYPE3" -%}
Item 3
{% elseif item.itemType equals "ITEM_TYPE4" -%}
Item 4
{% endif -%}
{% endfor %}
text after for loop preceded by blank line
I have over 100 templates with various nested template structures. I need to be able to have good control over the spacing and whitespace in the output. I will work on addressing this in the Pebble code once I am sure that this is an issue and not just a misunderstanding on my part.
I will be glad to work on this. I started looking at the code. I see that the whitespace control is done in the LexerImpl. I will try to make unit tests that isolate the issue at a lower level than the current template tests. Maybe I can make unit tests for the LexerImpl. Not sure yet.
I submitted a pull requests to add the tests that I made so far (which are template integration tests rather than true unit tests). No matter if this issue is addressed or not, I think these are useful tests. Will you review and accept the pull request or let me know if you have any issues with it?
This appears to be a design issue in that the Lexer processes the Whitespace Control Character (i.e. the dash character) and the remaining new line characters (i.e. the \n) are passed to the Parser as part of a Text Token. Therefore, when the Whitespace Control Character is used on an if statement, the output will be affected only if the condition in the if statement is true. For an if statement in a for loop, the new line characters in the output will vary depending on which iterations result in the body of the if statement being executed or not.
I think that the responsibilities between the Lexer and Parser need to be modified. My first thought would be for the Whitespace Control Character to be treated as a separate token (Operator Token maybe?) and for the new line characters to be included as separate TEXT tokens or perhaps a new type of token. In other words, all of the information would be passed to the Parser as Tokens in the Token Stream and so it would be the Parser's responsibility to remove the new line tokens or not based on the Whitespace Control Characters.
This is important to me. So, I will work on this and I will add unit tests for the Lexer and the Parser separately and possibly other tests at various levels. I would like to know if you have any concerns about me working on it. I will be open to feedback when I make pull requests. Do you want me to make pull requests for small changes? Would that be easier to review?
The following Lexer unit tests that I have written shows how the whitespace control character on an if statement affects the Text Token, which will be passed to the Parser.
@Test
public void testIfStatement() {
StringBuilder stringBuilder = new StringBuilder();
stringBuilder
.append("{% if item equals \"whatever\" %}\n")
.append("some text\n")
.append("{% endif %}");
Loader<String> loader = new StringLoader();
Reader templateReader = loader.getReader(stringBuilder.toString());
TokenStream tokenStream = this.lexer.tokenize(templateReader, TEMPLATE_NAME);
assertThat(tokenStream.getTokens().size()).isEqualTo(11);
int i = 0;
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_START);
assertThat(tokenStream.peek(i++).getValue()).isNull();
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("if");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("item");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.OPERATOR);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("equals");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.STRING);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("whatever");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_END);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("%}");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.TEXT);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("\nsome text\n");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_START);
assertThat(tokenStream.peek(i++).getValue()).isNull();
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("endif");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_END);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("%}");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EOF);
}
@Test
public void testIfStatementWithWhitespaceControl() {
StringBuilder stringBuilder = new StringBuilder();
stringBuilder
.append("{% if item equals \"whatever\" -%}\n")
.append("some text\n")
.append("{%- endif %}");
Loader<String> loader = new StringLoader();
Reader templateReader = loader.getReader(stringBuilder.toString());
TokenStream tokenStream = this.lexer.tokenize(templateReader, TEMPLATE_NAME);
assertThat(tokenStream.getTokens().size()).isEqualTo(11);
int i = 0;
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_START);
assertThat(tokenStream.peek(i++).getValue()).isNull();
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("if");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("item");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.OPERATOR);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("equals");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.STRING);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("whatever");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_END);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("%}");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.TEXT);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("some text");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_START);
assertThat(tokenStream.peek(i++).getValue()).isNull();
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.NAME);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("endif");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EXECUTE_END);
assertThat(tokenStream.peek(i++).getValue()).isEqualTo("%}");
assertThat(tokenStream.peek(i).getType()).isEqualTo(Type.EOF);
}
Great! I submitted a pull request for a few "template/functional" tests previously and that pull request is still pending. Would you be willing to approve that pull request to start with? (Or, let me know if you have any issues with it).
https://github.com/PebbleTemplates/pebble/pull/477
I will then submit another pull requests for these Lexer Impl tests and then continue working on this change probably with more tests at various levels before I get into changing the processing.
I checked the pull request comments again just now. The last comment that I see from you was when the build failed 15 days ago (which is now passing).
