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 Nested Map Highlighting #925

Merged
merged 3 commits into from
Feb 9, 2022
Merged

Fix Nested Map Highlighting #925

merged 3 commits into from
Feb 9, 2022

Conversation

dbanck
Copy link
Member

@dbanck dbanck commented Feb 8, 2022

This PR fixes the incorrect highlighting of nested maps.

Background

We’re using a two-pattern rule for locating objects. The begin key is a regular expression sought initially. Then, TextMate will keep walking the document until it matches the end expression and stops. Everything between the two found lines is considered the body and can be further matched with patterns.

In the example, the object starts in line 1 and ends with the first closing bracket in line 18. So our generic attribute_definition rule matches the first part of line 19, after which another object starts and ends in line 21. Most of the other brackets have no scope attached.

2022-02-07 19 01 41

Solution

Having the objects pattern include itself allows deeply nested objects and results in a new meta.braces.terraform scope for each nested level.

Please have a look at the commits for an explanation of the different additions and updates to the snapshot tests.

UX Impact

This showcases all impacted areas, sorted by snapshot coverage.

Nested maps

Before
CleanShot 2022-02-08 at 17 37 03

After
CleanShot 2022-02-08 at 17 37 15

Basic

Before
CleanShot 2022-02-08 at 17 40 52

After
CleanShot 2022-02-08 at 17 40 40

Data Sources

Before
CleanShot 2022-02-08 at 17 41 51

After
CleanShot 2022-02-08 at 17 41 59

Expressions For

Before
CleanShot 2022-02-08 at 17 02 05

After
CleanShot 2022-02-08 at 17 02 15

I've created #926 for further highlighting improvements.

Variables Input

Before
CleanShot 2022-02-08 at 17 43 51

After
CleanShot 2022-02-08 at 17 43 40

Variables Local

Before
CleanShot 2022-02-08 at 17 45 02

After
CleanShot 2022-02-08 at 17 45 09

Fixes #675.

This adds a terraform file containing nested maps and a snapshot
test capturing the broken highlighting
This commit adds `objects` as an include to `objects`. It allows
deeply nested objects and results in a new `meta.braces.terraform`
scope for each nested level.
This changes the scope for object keys from `string.unquoted.terraform`
to `variable.other.readwrite.terraform`, allowing them to be highlighted
in line with other variables.

With the update of the `keyword.operator` the `=` inside nested maps is
also highlighted in line with variable assignments. This change also
applied to inline for expressions, so we had to extend the pattern
for matching `=>`.

Matching `=>` here is only a quick-fix. When we improve the detection
of inline `for` loops, it can be removed.
@dbanck dbanck added bug Something isn't working syntax labels Feb 8, 2022
@dbanck dbanck requested a review from a team February 8, 2022 17:00
Copy link
Contributor

@jpogran jpogran left a comment

Choose a reason for hiding this comment

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

Awesome work!

@dbanck dbanck merged commit 8ad327c into main Feb 9, 2022
@dbanck dbanck deleted the bug-nested-map-syntax branch February 9, 2022 17:30
@jpogran jpogran added this to the 2.20.0 milestone Feb 18, 2022
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect colouring with nested maps
2 participants