-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: add OpenEdge ABL file extensions support for codebase indexing #7522
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
- Added .p, .i, and .w extensions to tree-sitter extensions list - Added these extensions to fallback chunking list since no WASM parser is available - This enables indexing of OpenEdge ABL files in codebase indexing feature Fixes #7519
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.
Reviewing my own code is like grading my own homework - I already know where I cut corners.
| // Visual Basic .NET | ||
| "vb", | ||
| // OpenEdge ABL | ||
| "p", |
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.
Is this formatting intentional? Other multi-extension languages like C/C++ (lines 41-46) use a single comment above the extensions. Consider:
| ".scala", // Scala - uses fallback chunking instead of Lua query workaround | ||
| ".swift", // Swift - uses fallback chunking due to parser instability | ||
| ".p", // OpenEdge ABL - no dedicated WASM parser | ||
| ".i", // OpenEdge ABL include file - no dedicated WASM parser |
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.
These comments could be more descriptive to help future maintainers. For example, instead of just "OpenEdge ABL include file", consider "OpenEdge ABL include file - contains shared definitions and procedures". Would this additional context be helpful?
Missing Test CoverageI noticed this PR doesn't include tests for the new OpenEdge ABL extensions. The existing test file Consider adding test cases to verify:
This would ensure the implementation is robust and prevent regressions. |
|
in addition to ".p", ".i", ".w" you should also include : ".cls" and ".df" extensions. |
|
This won't work, our tree-sitter package doesn't support these formats |
Summary
This PR addresses Issue #7519 by adding support for OpenEdge ABL file extensions (.p, .i, .w) to the codebase indexing feature.
Problem
Users reported that OpenEdge ABL files were being ignored during codebase indexing, with only .md and .json files being properly indexed. This prevented the indexing feature from working with OpenEdge ABL projects.
Solution
Added OpenEdge ABL file extensions to:
Changes
src/services/tree-sitter/index.tssrc/services/code-index/shared/supported-extensions.tsTesting
Impact
This change enables users working with OpenEdge ABL projects to use the codebase indexing feature, allowing their .p (program), .i (include), and .w (window) files to be properly indexed and searchable.
Fixes #7519
Important
Adds support for OpenEdge ABL file extensions (.p, .i, .w) to codebase indexing by updating
index.tsandsupported-extensions.ts.extensionsinindex.tsto include .p, .i, .w for recognition.fallbackExtensionsinsupported-extensions.tsto include .p, .i, .w for fallback chunking.This description was created by
for 48d24e8. You can customize this summary. It will automatically update as commits are pushed.