-
Notifications
You must be signed in to change notification settings - Fork 321
Feat(lsp): Add find all and go to references support for Macros #4692
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
Conversation
benfdking
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work! Just little things I would do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for finding and navigating to macro references in both the VSCode extension and the LSP server.
- Introduces Playwright tests for “Go to References” and “Find All References” on SQLMesh macros in the VSCode extension
- Extends LSP reference resolver with
get_macro_find_all_references, integrates it into the genericget_all_references - Updates sample SQL files and existing tests to include a new
another_columnfor lineage/forking consistency
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vscode/extension/tests/find_references.spec.ts | Added end-to-end tests for macro reference navigation |
| tests/web/test_lineage.py | Updated SQL in test to include new another_column |
| tests/test_forking.py | Same SQL update for parallel load test |
| tests/lsp/test_reference_macro_find_all.py | New unit tests for LSP macro find-all-references logic |
| tests/integrations/github/cicd/test_integration.py | Erroneous diff markers inserted in CI integration test |
| sqlmesh/lsp/reference.py | Implemented get_macro_find_all_references and hooked it in get_all_references |
| examples/sushi/models/customers.sql | Added a macro usage example (@ADD_ONE(1) AS another_column) |
Comments suppressed due to low confidence (4)
tests/integrations/github/cicd/test_integration.py:1685
- The diff in this test file appears malformed with a stray
+++marker. Please remove or correct these diff markers so the test can parse correctly.
+++
sqlmesh/lsp/reference.py:590
- [nitpick] The parameter is named
lint_contextbut the rest of this module useslsp_context. Consider renaming for consistency and clarity.
def get_macro_find_all_references(
vscode/extension/tests/find_references.spec.ts:517
- It looks like
GO_TO_REFERENCES_KEY(and similarlyFIND_ALL_REFERENCES_KEY) are used but not imported. Please add the appropriate import statements at the top of the file.
await window.keyboard.press(GO_TO_REFERENCES_KEY)
tests/lsp/test_reference_macro_find_all.py:110
- [nitpick] There's an extra space before the period in this docstring. You can tighten it to
"Test finding references to @SQL_LITERAL macro.".
"""Test finding references to @SQL_LITERAL macro ."""
967d0e6 to
ceacc7b
Compare
Building on #4652 and #4680, this update adds "Find All References" and "Go to References" support for macros in the SQLMesh VSCode extension.