Skip to content

Extend "Go to Definition" to cover class member definitions.#405

Open
isc-klu wants to merge 3 commits into
intersystems:masterfrom
isc-klu:automatic-symbol-reference
Open

Extend "Go to Definition" to cover class member definitions.#405
isc-klu wants to merge 3 commits into
intersystems:masterfrom
isc-klu:automatic-symbol-reference

Conversation

@isc-klu

@isc-klu isc-klu commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Fix #397

Go to Definition wasn't working for class member definition. For example, when users presses F12 on Foobar of the following code, they see "No definition found.” Fixing this issue also extends automatic symbol references to class members.

ClassMethod Foobar() {
}

@isc-bsaviano

Copy link
Copy Markdown
Collaborator

@isc-klu Can you explain how this addresses the issue? The code looks fine but I am not sure how the automatic symbol reference chat feature definition gets to this code.

@isc-klu

isc-klu commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator Author

It’s not clear from the VSCode documentation what is required for automatic symbol references to work. However, @hsyhhssyy’s comments in #397 suggest that the lack of automatic symbol references for members is related to the following issues:

As noted by @hsyhhssyy, automatic symbol references do work for class symbols. Since we currently don’t support workspace symbols (including for class symbols), the second issue is likely unrelated.

With that in mind, I focused on addressing the first issue. Fixing Go to Definition for member definitions also resolves the automatic symbol references problem.

Given this, it would be more accurate to position this PR as a fix for Go to Definition, with improved automatic symbol references as a side effect. I’ll update the PR description shortly.

@isc-klu isc-klu changed the title Automatic Symbol Reference for class members Extend "Go to Definition" to cover class member definitions. Jun 1, 2026
@isc-klu

isc-klu commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

@isc-bsaviano — following up on our discussion about short‑circuiting the special case: my latest commit implements that change.

A couple of notes:

  • I added a type annotation to support the planned optimization. I believe it improves readability/maintainability, so I’ve left it in.
  • The swap of two if branches in the old code (around L432) is no longer functionally relevant. However, I think the new ordering is more maintainable—putting the shorter branch first makes it easier to see the structure—so I kept the updated order.

Comment thread server/src/providers/definition.ts Outdated
) {
// This is a class member definition
const range = findFullRange(params.position.line, parsed, i, symbolstart, symbolend);
return [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to use classMemberLocationLink() for this. It will need some modification for this special case where we know the definition is in the current class.

@isc-klu isc-klu Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you meant findMemberInCurrentClass, which constructs a location link from an already available document, rather than classMemberLocationLink, which first locates the relevant document on a server. Using the latter here would be unnecessary overhead since we already have the document.

Could you check whether the new commit (2bd2ef7) aligns with your expectations?

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.

Suggestion: Improve Symbol Definitions in ObjectScript LS for Better VS Code Integration

2 participants