Skip to content

feat(vscode): diagnostic for bad config.yaml error#4945

Merged
benfdking merged 1 commit intomainfrom
testing_bad_config_just_yaml
Jul 9, 2025
Merged

feat(vscode): diagnostic for bad config.yaml error#4945
benfdking merged 1 commit intomainfrom
testing_bad_config_just_yaml

Conversation

@benfdking
Copy link
Contributor

@benfdking benfdking commented Jul 9, 2025

  • return error
  • filter diagnostics so it doesn't show in wrong place
image

@benfdking benfdking force-pushed the testing_bad_config_just_yaml branch from 11ec037 to 8d996de Compare July 9, 2025 19:21
@benfdking benfdking requested a review from Copilot July 9, 2025 19:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for reporting and filtering diagnostics when a malformed config.yaml is present in a workspace, ensures the YAML parser is available in tests, and includes a new end-to-end test to cover the invalid-config scenario.

  • Add yaml dependency and import execAsync in tests
  • Filter LSP diagnostics so only the relevant URI shows the config error
  • Track and attach config.yaml path to ConfigError for accurate error location

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
vscode/extension/tests/broken_project.spec.ts New test suite for bad config.yaml handling and error display
vscode/extension/package.json Added "yaml": "^2.8.0" dependency for YAML parsing in tests
sqlmesh/lsp/main.py Filter configuration diagnostics by matching the URI value
sqlmesh/core/config/loader.py Record yaml_config_path and pass it as location in ConfigError
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)

vscode/extension/tests/broken_project.spec.ts:316

  • The test asserts the error text appears but doesn't verify that the diagnostic is scoped to config.yaml. Consider adding an assertion on the Problems view entry to confirm the file path or URI, ensuring the diagnostic filtering works as intended.
      .isVisible({ timeout: 1_000 })

@benfdking benfdking force-pushed the testing_bad_config_just_yaml branch from 8d996de to 55f6387 Compare July 9, 2025 19:47
- return error
- filter diagnostics so it doesn't show in wrong place
@benfdking benfdking force-pushed the testing_bad_config_just_yaml branch from 55f6387 to b89646a Compare July 9, 2025 19:58
@benfdking benfdking merged commit 8421488 into main Jul 9, 2025
26 checks passed
@benfdking benfdking deleted the testing_bad_config_just_yaml branch July 9, 2025 20:22
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.

3 participants