Skip to content

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

podcherklife
Copy link

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/imports
They can be generated by build script instead, but that would require java dependency during build which might be undesirable.

@burner1024
Copy link
Member

burner1024 commented Aug 1, 2024

That's cool.
I've been meaning to get to ANTLR4 next, in particular as the road to autoformatting.

Language-specific features I prefer to put into corresponding weidu.ts or fallout.ts files. Or, failing that, route through Galactus

// the only language with external headers dir
if (l.id == "fallout-ssl") {
language = new Language(

But there's been no need for language-specific lookup. I guess you can pass langId to symbolAtPosition and handle the switch there, in common.ts
Doesn't current symbolAtPosition return correct symbol, though?

They can be generated by build script instead, but that would require java dependency during build which might be undesirable.

Yes, will need the generation script. Not necessarily at build time, but need to have a way to regenerate this automatically.
See https://github.com/BGforgeNet/VScode-BGforge-MLS/blob/master/scripts/ie-update.sh for an example. I just checkout upstream repos into ./external and pull data. Kind of horrible, but better than nothing, upstreams don't change often.

Also seems ESlint is plenty angry.

@burner1024 burner1024 changed the title Added go-to-defintion feature to weidu .d files Added go-to-definition feature to weidu .d files Aug 1, 2024
@burner1024 burner1024 requested a review from Copilot June 12, 2025 10:27
Copy link

@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 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 the onDefinition handler.
  • Enables definition and udf features for weidu-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 wraps findSymbolAtPosition. 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 to ErrorResistantVisitor 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") {
Copy link
Preview

Copilot AI Jun 12, 2025

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.

Suggested change
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] == '%') {
Copy link
Preview

Copilot AI Jun 12, 2025

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] == '%')).

Suggested change
} 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.

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