Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive testing harness for GitHub Actions workflow expression syntax highlighting, addressing several long-standing grammar bugs. It extracts and refines expression grammar improvements from PR #468 into a focused, testable unit with fixture-based regression tests and detailed documentation for future maintenance.
Changes:
- Adds fixture-based test infrastructure and Jest test suite for expression grammar regression testing
- Fixes multiple expression syntax highlighting issues: block
if:handling, trailing comments after quoted strings, multi-line inline expressions, and nested curly braces in strings - Provides comprehensive documentation for grammar triage workflow and test fixture conventions
- Updates test import style to use
@jest/globalsfor consistency with ESM configuration
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/workflow/syntax/syntax-test-utils.ts |
New test utilities for loading grammars/fixtures and parsing GitHub Actions expressions |
src/workflow/syntax/expression-syntax.test.ts |
Comprehensive Jest test suite covering expression grammar edge cases and regressions |
src/workflow/syntax/fixtures/*.yml |
Six YAML test fixtures covering if-expressions, nested braces, multi-line expressions, and comment handling |
src/workflow/syntax/README.md |
Documentation for syntax highlighting triage process and fixture-based testing patterns |
language/syntaxes/expressions.tmGrammar.json |
Refactored expression grammar with support for multi-line expressions, block if-syntax, and improved comment handling |
language/README.md |
Brief guide pointing to syntax testing documentation |
src/secrets/index.test.ts |
Updated Jest import style to match new test conventions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey @ericsciple: First, I apologize in advance for the two direct tags on you (with almost identical text). The main reason for two incoming pings is that I wanted to allow the PRs to stand on their own as much as y'all want, and so I wanted to separate the discussion but let the team have full context. I normally try not to hit people up directly but since these two PRs (#570 & #571) touch on so many issues, there wasn't really a single thread for me to extend the conversation, and I wasn't quite sure how you would like me to proceed. The original issue I wanted to discuss was embedded syntax highlighting. I found #194 that discussed that, so I figured I'd move ahead on that one. In the process of building that, I wanted a bit more confidence in my changes, so I put in some testing components. That made the PR large enough that I didn't feel comfortable moving forward without splitting it into two:
|
Is your feature request related to a problem? Please describe.
Syntax-highlighting regressions in GitHub Actions workflow expressions are hard to diagnose and easy to reintroduce. This codebase did not have testing around these, and there have been several errors and a few regressions dealt with and managed in Issues.
Describe the solution you'd like
This PR adds a small workflow syntax test harness, fixture-based expression coverage, and the expression grammar changes needed for those cases to pass.
The expression grammar changes in this PR are extracted from and based on prior work by @cdce8p in #468, including fixes for block
if:handling, comment-relatedifbehavior, multi-line inline expressions, and nested-curly-brace expression parsing.My additions in this PR are the supporting work needed to land and maintain that logic in a smaller reviewable unit:
Additional context
Shout-out to @cdce8p for the original expression grammar work in #468, which this PR builds on and extracts into a smaller reviewable unit.
This PR is the first half of a split from a larger change set. It intentionally contains only the expression grammar changes and the supporting regression-test/documentation work. Embedded syntax highlighting for
github-scriptandrunblocks will be submitted separately (#571).Validation:
npm test