Skip to content

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

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

Merged
merged 1 commit into from
Jul 9, 2025

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 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.

2 participants