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.

Sign up for GitHub

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

Jump to bottom

fix(template): properly handle escaping of interpolation/directive markers #247

Merged
merged 1 commit into from Jun 15, 2023
Merged

fix(template): properly handle escaping of interpolation/directive markers #247

merged 1 commit into from Jun 15, 2023

Conversation

martinohmann
Copy link
Owner

Fixes #242
Closes #243

This fixes the handling of ${ and %{ markers in template literals.

When a HCL template literal contains escaped interpolation sequence or directive control flow start markers ($${ and %%{ respectively), they will be automatically unescaped while parsing now.

Additionally, formatting a Element::Literal containing ${ or %{ as HCL will now correctly emit $${ and %%{.

@martinohmann
fix(template): properly handle escaping of interpolation/directive ma…
dcde671 
…rkers

Fixes #242
Closes #243

This fixes the handling of `${` and `%{` markers in template literals.

When a HCL template literal contains escaped interpolation sequence or
directive control flow start markers (`$${` and `%%{` respectively),
they will be automatically unescaped while parsing now.

Additionally, formatting a `Element::Literal` containing `${` or `%{` as
HCL will now correctly emit `$${` and `%%{`.
@martinohmann martinohmann merged commit 69ad800 into main Jun 15, 2023
5 checks passed
@martinohmann martinohmann mentioned this pull request Jun 15, 2023
Closed
@github-actions github-actions bot mentioned this pull request Jun 15, 2023
Merged
@vlad-ivanov-name
Copy link

Thanks! Would the user be able to tell whether the ${ came from the escaped sequence of from one of variables expansion in this implementation?

@martinohmann
Copy link
Owner Author

What do you mean? If you see a ${ in a Element::Literal, it always gets escaped when encoding zu HCL. Conversely, when parsing HCL it means that it was escaped in the source input when it ends up in a literal. Unescaped ${ always produce Element::Interpolation.

@vlad-ivanov-name
Copy link

Right, that makes sense

@martinohmann martinohmann mentioned this pull request Jun 16, 2023
Closed
martinohmann added a commit that referenced this pull request Jun 16, 2023
@martinohmann
fix(string): properly handle escaping of interpolation/directive marker
9561868 
Closes #248

This is the same fix as made in #247 for template literals, just for
strings.
@martinohmann martinohmann mentioned this pull request Jun 16, 2023
Merged
martinohmann added a commit that referenced this pull request Jun 16, 2023
@martinohmann
fix(string): properly handle escaping of interpolation/directive mark…
e0c86f1 
…er (#249)

Closes #248

This is the same fix as made in #247 for template literals, just for
strings.
@martinohmann martinohmann deleted the escaped-markers branch June 16, 2023 18:27
martinohmann added a commit that referenced this pull request Jun 18, 2023
@martinohmann
perf(parser): handle unescaping of escaped markers in parser code dir…
ff0de24 
…ectly

The changes in #247 and
#249 enabled some
improvements to the string parsing code which this change implements.

Namely, the unescaping of escaped template markers (`$${` -> `${` and `%%{` ->
`%{`) is done directly when they are encountered now. This speeds up
common cases quite a bit.

Before:

```
parse/hcl-edit/deeply_nested.tf
                        time:   [19.631 µs 19.647 µs 19.668 µs]
                        thrpt:  [35.929 MiB/s 35.968 MiB/s 35.997 MiB/s]
parse/hcl-edit/large.tf time:   [2.1344 ms 2.1389 ms 2.1445 ms]
                        thrpt:  [38.015 MiB/s 38.114 MiB/s 38.194 MiB/s]
parse/hcl-edit/medium.tf
                        time:   [449.06 µs 451.22 µs 453.42 µs]
                        thrpt:  [31.667 MiB/s 31.821 MiB/s 31.975 MiB/s]
parse/hcl-edit/small.tf time:   [27.485 µs 27.600 µs 27.744 µs]
                        thrpt:  [34.133 MiB/s 34.311 MiB/s 34.456 MiB/s]
```

After:

```
parse/hcl-edit/deeply_nested.tf
                        time:   [18.461 µs 18.488 µs 18.526 µs]
                        thrpt:  [38.146 MiB/s 38.223 MiB/s 38.280 MiB/s]
                 change:
                        time:   [-5.8990% -4.6557% -2.0981%] (p = 0.00 < 0.05)
                        thrpt:  [+2.1431% +4.8830% +6.2688%]
                        Performance has improved.
parse/hcl-edit/large.tf time:   [1.7640 ms 1.7699 ms 1.7787 ms]
                        thrpt:  [45.833 MiB/s 46.061 MiB/s 46.216 MiB/s]
                 change:
                        time:   [-17.613% -17.253% -16.777%] (p = 0.00 < 0.05)
                        thrpt:  [+20.160% +20.850% +21.379%]
                        Performance has improved.
parse/hcl-edit/medium.tf
                        time:   [407.78 µs 408.77 µs 409.99 µs]
                        thrpt:  [35.022 MiB/s 35.127 MiB/s 35.212 MiB/s]
                 change:
                        time:   [-9.9006% -9.4090% -8.9185%] (p = 0.00 < 0.05)
                        thrpt:  [+9.7918% +10.386% +10.988%]
                        Performance has improved.
parse/hcl-edit/small.tf time:   [24.139 µs 24.249 µs 24.385 µs]
                        thrpt:  [38.835 MiB/s 39.053 MiB/s 39.230 MiB/s]
                 change:
                        time:   [-12.375% -12.060% -11.751%] (p = 0.00 < 0.05)
                        thrpt:  [+13.316% +13.713% +14.123%]
                        Performance has improved.
```
martinohmann added a commit that referenced this pull request Jun 18, 2023
@martinohmann
perf(parser): handle unescaping of escaped markers in parser code dir…
b7e7de1 
…ectly

The changes in #247 and
#249 enabled some
improvements to the string parsing code which this change implements.

Namely, the unescaping of escaped template markers (`$${` -> `${` and `%%{` ->
`%{`) is done directly when they are encountered now. This speeds up
common cases quite a bit.

Before:

```
parse/hcl-edit/deeply_nested.tf
                        time:   [19.631 µs 19.647 µs 19.668 µs]
                        thrpt:  [35.929 MiB/s 35.968 MiB/s 35.997 MiB/s]
parse/hcl-edit/large.tf time:   [2.1344 ms 2.1389 ms 2.1445 ms]
                        thrpt:  [38.015 MiB/s 38.114 MiB/s 38.194 MiB/s]
parse/hcl-edit/medium.tf
                        time:   [449.06 µs 451.22 µs 453.42 µs]
                        thrpt:  [31.667 MiB/s 31.821 MiB/s 31.975 MiB/s]
parse/hcl-edit/small.tf time:   [27.485 µs 27.600 µs 27.744 µs]
                        thrpt:  [34.133 MiB/s 34.311 MiB/s 34.456 MiB/s]
```

After:

```
parse/hcl-edit/deeply_nested.tf
                        time:   [18.461 µs 18.488 µs 18.526 µs]
                        thrpt:  [38.146 MiB/s 38.223 MiB/s 38.280 MiB/s]
                 change:
                        time:   [-5.8990% -4.6557% -2.0981%] (p = 0.00 < 0.05)
                        thrpt:  [+2.1431% +4.8830% +6.2688%]
                        Performance has improved.
parse/hcl-edit/large.tf time:   [1.7640 ms 1.7699 ms 1.7787 ms]
                        thrpt:  [45.833 MiB/s 46.061 MiB/s 46.216 MiB/s]
                 change:
                        time:   [-17.613% -17.253% -16.777%] (p = 0.00 < 0.05)
                        thrpt:  [+20.160% +20.850% +21.379%]
                        Performance has improved.
parse/hcl-edit/medium.tf
                        time:   [407.78 µs 408.77 µs 409.99 µs]
                        thrpt:  [35.022 MiB/s 35.127 MiB/s 35.212 MiB/s]
                 change:
                        time:   [-9.9006% -9.4090% -8.9185%] (p = 0.00 < 0.05)
                        thrpt:  [+9.7918% +10.386% +10.988%]
                        Performance has improved.
parse/hcl-edit/small.tf time:   [24.139 µs 24.249 µs 24.385 µs]
                        thrpt:  [38.835 MiB/s 39.053 MiB/s 39.230 MiB/s]
                 change:
                        time:   [-12.375% -12.060% -11.751%] (p = 0.00 < 0.05)
                        thrpt:  [+13.316% +13.713% +14.123%]
                        Performance has improved.
```

Please note that I didn't bother to implement the same improvement for
the `hcl-rs` parser because it will be removed in the future and then
just uses `hcl-edit` under the hood for parsing.
@martinohmann martinohmann mentioned this pull request Jun 18, 2023
Merged
martinohmann added a commit that referenced this pull request Jun 18, 2023
@martinohmann
perf(parser): handle unescaping of escaped markers in parser code dir…
45036c5 
…ectly (#251)

The changes in #247 and
#249 enabled some
improvements to the string parsing code which this change implements.

Namely, the unescaping of escaped template markers (`$${` -> `${` and `%%{` ->
`%{`) is done directly when they are encountered now. This speeds up
common cases quite a bit.

Before:

```
parse/hcl-edit/deeply_nested.tf
                        time:   [19.631 µs 19.647 µs 19.668 µs]
                        thrpt:  [35.929 MiB/s 35.968 MiB/s 35.997 MiB/s]
parse/hcl-edit/large.tf time:   [2.1344 ms 2.1389 ms 2.1445 ms]
                        thrpt:  [38.015 MiB/s 38.114 MiB/s 38.194 MiB/s]
parse/hcl-edit/medium.tf
                        time:   [449.06 µs 451.22 µs 453.42 µs]
                        thrpt:  [31.667 MiB/s 31.821 MiB/s 31.975 MiB/s]
parse/hcl-edit/small.tf time:   [27.485 µs 27.600 µs 27.744 µs]
                        thrpt:  [34.133 MiB/s 34.311 MiB/s 34.456 MiB/s]
```

After:

```
parse/hcl-edit/deeply_nested.tf
                        time:   [18.461 µs 18.488 µs 18.526 µs]
                        thrpt:  [38.146 MiB/s 38.223 MiB/s 38.280 MiB/s]
                 change:
                        time:   [-5.8990% -4.6557% -2.0981%] (p = 0.00 < 0.05)
                        thrpt:  [+2.1431% +4.8830% +6.2688%]
                        Performance has improved.
parse/hcl-edit/large.tf time:   [1.7640 ms 1.7699 ms 1.7787 ms]
                        thrpt:  [45.833 MiB/s 46.061 MiB/s 46.216 MiB/s]
                 change:
                        time:   [-17.613% -17.253% -16.777%] (p = 0.00 < 0.05)
                        thrpt:  [+20.160% +20.850% +21.379%]
                        Performance has improved.
parse/hcl-edit/medium.tf
                        time:   [407.78 µs 408.77 µs 409.99 µs]
                        thrpt:  [35.022 MiB/s 35.127 MiB/s 35.212 MiB/s]
                 change:
                        time:   [-9.9006% -9.4090% -8.9185%] (p = 0.00 < 0.05)
                        thrpt:  [+9.7918% +10.386% +10.988%]
                        Performance has improved.
parse/hcl-edit/small.tf time:   [24.139 µs 24.249 µs 24.385 µs]
                        thrpt:  [38.835 MiB/s 39.053 MiB/s 39.230 MiB/s]
                 change:
                        time:   [-12.375% -12.060% -11.751%] (p = 0.00 < 0.05)
                        thrpt:  [+13.316% +13.713% +14.123%]
                        Performance has improved.
```

Please note that I didn't bother to implement the same improvement for
the `hcl-rs` parser because it will be removed in the future and then
just uses `hcl-edit` under the hood for parsing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

$${ should be evaluated to ${
2 participants
@martinohmann @vlad-ivanov-name