Skip to content

Add keepTrailingNewline option to LegacyOverrides to match Python Jinja2 default behaviour#1311

Open
jmoraleda wants to merge 1 commit intoHubSpot:masterfrom
jmoraleda:keep_trailing_newline
Open

Add keepTrailingNewline option to LegacyOverrides to match Python Jinja2 default behaviour#1311
jmoraleda wants to merge 1 commit intoHubSpot:masterfrom
jmoraleda:keep_trailing_newline

Conversation

@jmoraleda
Copy link
Copy Markdown

Description:

By default, Python Jinja2 strips a single trailing newline from rendered output (keep_trailing_newline=False). Jinjava has historically preserved trailing newlines, causing a subtle but consistent difference when porting templates from Python Jinja2.

This PR adds a keepTrailingNewline flag to LegacyOverrides that allows users to opt into Python-compatible behaviour.

Changes:

  • LegacyOverrides: adds isKeepTrailingNewline(), defaulting to true (preserve existing Jinjava behaviour — no impact on current users).
  • JinjavaInterpreter: applies the strip in stripTrailingNewlineIfNeeded(), called at the normal return points of the private render() method. Early-exit error paths (OutputTooBigException, CollectionTooBigException) are intentionally unaffected.
  • TrailingNewlineTest: covers both flag values, the NONE/THREE_POINT_0/ALL presets, and edge cases (no trailing newline, multiple trailing newlines, rendered expressions).

Usage:

To opt into Python-compatible behaviour:

JinjavaConfig config = JinjavaConfig.newBuilder()
    .withLegacyOverrides(
        LegacyOverrides.newBuilder()
            .withKeepTrailingNewline(false)
            .build()
    )
    .build();

Compatibility note:

keepTrailingNewline is set to true (legacy behaviour) in THREE_POINT_0 to avoid breaking existing tests. It is set to false in ALL. Maintainers may wish to move it to false in THREE_POINT_0 as well if they consider stripping the trailing newline to be part of the canonical Python-compatible behaviour set — the change is intentionally conservative here to leave that decision to the maintainers.

Copy link
Copy Markdown
Contributor

@jasmith-hs jasmith-hs left a comment

Choose a reason for hiding this comment

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

Thanks @jmoraleda for the PR. Could you change it so that the default is false like the other legacy override options and rename it something like stripTrailingNewlines?
Also, as Jinjava 3.0 has not been published yet, so this option can be turned on for 3.0 so that the default behaviour of Jinjava 3.0 is as close to Jinja as possible

@jmoraleda
Copy link
Copy Markdown
Author

Thank you @jasmith-hs :

  1. I am ok changing it to stripTrailingNewlines and reversing the logic. But since in Python jinja they use

    image

    Can you please confirm you still want to change the name and reverse the logic?

  2. I am happy to include this change in THREE_POINT_0. Since THREE_POINT_0 is the default used for testing, that breaks some of the existing tests. For tests that break, do you prefer me to change the expected output of the tests to match THREE_POINT_0 default behavior, or do you prefer to modify the test to use a modified configuration that preserves the older behavior?

  3. Should I also modify fix: treat backslash as escape character only inside quoted strings, matching Jinja2 behaviour (fixes #1304) #1306 to add that feature to THREE_POINT_0 configuration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants