Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Add tests for HEREDOC patterns #842

Merged
merged 2 commits into from
Oct 19, 2023
Merged

Add tests for HEREDOC patterns #842

merged 2 commits into from
Oct 19, 2023

Conversation

sambostock
Copy link
Contributor

@sambostock sambostock commented Oct 17, 2023

Motivation

We have many patterns for embedded languages in HEREDOCs, and manually ensuring they are correct can be problematic, especially as we want to add new ones (e.g. YAML).

Implementation

This introduces tests to enforce their correctness, generated from an easier to manage minimal config. It also adds tests to actually check that the grammar results in parsing the expected tokens.

Automated Tests

The first commit consists of only tests. 📈
The second commit is a TDD change to capitalization.

Manual Tests

If heredoc highlighting continues to work, everything is working.

@sambostock sambostock requested a review from a team as a code owner October 17, 2023 16:17
@sambostock sambostock force-pushed the heredoc-pattern-tests branch 2 times, most recently from 77dcd68 to 341909a Compare October 17, 2023 18:28
@sambostock sambostock marked this pull request as draft October 17, 2023 18:28
@sambostock sambostock force-pushed the heredoc-pattern-tests branch 2 times, most recently from 5ca6c02 to 37da827 Compare October 18, 2023 03:41
@sambostock sambostock marked this pull request as ready for review October 18, 2023 03:42
We have many patterns for embedded languages in HEREDOCs, and manually
ensuring they are correct can be problematic.

This introduces tests to enforce their correctness, generated from an
easier to manage minimal config.
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Very nice! Just a question about the TS config.

@@ -3,7 +3,7 @@
"module": "commonjs",
"target": "ES2020",
"outDir": "out",
"lib": ["ES2020"],
"lib": ["dom", "ESNext"],
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • ESNext is needed for Array#toSorted
  • dom is needed for WebAssembly & Response types, which are used by node_modules/vscode-oniguruma/main.d.ts1

AFAIK the code still ends up the same though, as target hasn't changed.

Footnotes

  1. See Separate WebAssembly types from lib.dom.d.ts into their own file microsoft/TypeScript-DOM-lib-generator#826 for discussion

Copy link
Member

Choose a reason for hiding this comment

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

I made a few tests and it seems like everything is fine after changing the targets. Let's move forward with this and I can make a preview extension release just to confirm the compilation works - before doing the stable release.

@sambostock sambostock merged commit 363cee2 into main Oct 19, 2023
@sambostock sambostock deleted the heredoc-pattern-tests branch October 19, 2023 19:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants