-
-
Notifications
You must be signed in to change notification settings - Fork 6
Added go-to-definition feature to weidu .d files #74
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
base: master
Are you sure you want to change the base?
Conversation
That's cool. Language-specific features I prefer to put into corresponding VScode-BGforge-MLS/server/src/galactus.ts Lines 231 to 233 in 96f48ca
But there's been no need for language-specific lookup. I guess you can pass
Yes, will need the generation script. Not necessarily at build time, but need to have a way to regenerate this automatically. Also seems ESlint is plenty angry. |
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
This PR adds a go-to-definition feature for .d
files in the Weidu language support by leveraging a simple ANTLR-based parser to collect definitions from open files and hooking it into the LSP definition handler.
- Introduces a specialized symbol finder for
weidu-d
in theonDefinition
handler. - Enables
definition
andudf
features forweidu-d
in language and Galactus configurations. - Adds
d.ts
with ANTLR visitor implementations to collect state definitions and transition targets for go-to-definition.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
server/src/server.ts | Imports and invokes a custom symbol finder for the weidu-d language in the definition hook |
server/src/language.ts | Registers loadFileData under the weidu-d language ID |
server/src/galactus.ts | Enables definition and udf features for weidu-d |
server/src/d.ts | Implements ANTLR-based parsing, symbol collection, and state-definition logic |
Comments suppressed due to low confidence (2)
server/src/server.ts:20
- [nitpick] The alias
findTransitionAtPosition
may be misleading since it wrapsfindSymbolAtPosition
. Consider using a name that reflects it finds symbols (e.g.,findSymbolAtPosition
) for consistency.
import { findSymbolAtPosition as findTransitionAtPosition } from "./d";
server/src/d.ts:68
- [nitpick] There's a typo in the class name
ErroResistantVisitor
. Consider renaming it toErrorResistantVisitor
for correct spelling and clarity.
abstract class DlgAwareErroResistantVisitor<T> extends AbstractParseTreeVisitor<T> implements DParserVisitor<T> {
@@ -271,6 +272,9 @@ connection.onDefinition((params) => { | |||
} | |||
const langId = textDoc.languageId; | |||
const text = textDoc.getText(); | |||
const symbol = symbolAtPosition(text, params.position); | |||
let symbol = symbolAtPosition(text, params.position); | |||
if (langId == "weidu-d") { |
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.
Use strict equality (===
) instead of loose equality (==
) when comparing langId
to avoid unintended type coercion.
if (langId == "weidu-d") { | |
if (langId === "weidu-d") { |
Copilot uses AI. Check for mistakes.
function cleanString(str: string) { | ||
if (str.startsWith('~~~~~')) { | ||
return str.substring(5, str.length - 5); | ||
} else if (str.length > 0 && str[0] == '~' || str[0] == "\"" || str[0] == '%') { |
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.
The else if
condition lacks grouping, so str[0] == '"'
or str[0] == '%'
will be evaluated even if str.length
is 0. Wrap the OR clauses in parentheses: else if (str.length > 0 && (str[0] == '~' || str[0] == '"' || str[0] == '%'))
.
} else if (str.length > 0 && str[0] == '~' || str[0] == "\"" || str[0] == '%') { | |
} else if (str.length > 0 && (str[0] == '~' || str[0] == "\"" || str[0] == '%')) { |
Copilot uses AI. Check for mistakes.
This PR adds limited go-to-defintion feature for weidu .d files.
Definitions are collected only from open files.
Go-to-definition works only for GOTO and EXTERN weidu commands.
Notes regarding implementation:
Holding clrl causes file to be re-parsed on every cursor move. I did not notice any visible impact on performance even for large files, but you mileage may vary.
I got a bit confused about how to implement a language-specific symbol lookup, and went the most direct way. Suggestions on how to properly improve that are welcome.
Files in
src/dparser/*
were generated from this grammar: https://gitlab.com/lapdu/lapdu-parser/-/tree/master/src/main/antlr4/importsThey can be generated by build script instead, but that would require java dependency during build which might be undesirable.