Fix cross-file go-to-definition navigation in Pyrefly Sandbox - Issue #1061

[hashiranhar]

Problem

Issue: #1061 Go-to-definition across files in the web editor was jumping to random locations in the current file
instead of navigating to the correct location in the target file.

Solution

Enhanced the go-to-definition system to return target file information:

1. Backend: Updated goto_definition() to return DefinitionResult struct with both range and
   target filename
2. Frontend: Modified Monaco definition provider to use correct target file URI instead of always
   using current file URI
3. Navigation: Added automatic file switching when definition is in a different file

Key Changes

• playground.rs: New DefinitionResult struct with filename field
• configured-monaco.ts: Fixed definition provider to use target file URI
• Sandbox.tsx: Added cross-file navigation support
• Added test test_cross_file_goto_definition for cross-file goto-definition functionality

Testing

• ✅ Cross-file navigation now works correctly
• ✅ Same-file navigation unchanged
• ✅ Added test coverage for new functionality

[kinto0]

Thank you so much!!

This change is great. Awesome test coverage.

[facebook-github-bot]

@kinto0 has imported this pull request. If you are a Meta employee, you can view this in D82452553.

[stroxler]

Review automatically exported from Phabricator review in Meta.

[kinto0]

manual testing reveals a few issues:

• hovering the function and pressing cmd ends up performing the go-to def (this isn't the case on the prod sandbox so I wonder if it was introduced here?)
• navigating across files does not seem to end up on the correct line

would you be able to do more manual testing to ensure this feature works correctly? sorry I didn't catch these earlier, I am not too familiar with the sandbox

[hashiranhar]

manual testing reveals a few issues:

* hovering the function and pressing cmd ends up performing the go-to def (this isn't the case on the prod sandbox so I wonder if it was introduced here?)

* navigating across files does not seem to end up on the correct line

would you be able to do more manual testing to ensure this feature works correctly? sorry I didn't catch these earlier, I am not too familiar with the sandbox

1. This is standard monaco editor behaviour I believe, did not code anything on my side, just that now the goto definition works hence so does the shortcut for it
2. Let me check this, my current logic just lands you on the file where the definition is stored I believe. Its a bit short sighted but I was just assuming at that time people would be using small imports to check out a sandbox. I can refine this tho

[facebook-github-bot]

@kinto0 has imported this pull request. If you are a Meta employee, you can view this in D82452553.

[kinto0]

Sorry about the delayed review!! I was at a conference and this dropped off my radar.

This is standard monaco editor behaviour I believe, did not code anything on my side, just that now the goto definition works hence so does the shortcut for it

VSCode has two apis: "get definition" and "go to definition". Get definition happens on cmd+hover in order to preview the location, but we only want to "go" on click. I imagine monaco is similar, and we have hooked into the "get" instead of the "go to". Do you think you could attempt this?

Let me check this, my current logic just lands you on the file where the definition is stored I believe. Its a bit short sighted but I was just assuming at that time people would be using small imports to check out a sandbox. I can refine this tho

I think it's necessary to bring them to the line/column if that would work. otherwise, finding the import is just as good since the user likely only has a few tabs open in the sandbox

[hashiranhar]

Sorry about the delayed review!! I was at a conference and this dropped off my radar.

This is standard monaco editor behaviour I believe, did not code anything on my side, just that now the goto definition works hence so does the shortcut for it

VSCode has two apis: "get definition" and "go to definition". Get definition happens on cmd+hover in order to preview the location, but we only want to "go" on click. I imagine monaco is similar, and we have hooked into the "get" instead of the "go to". Do you think you could attempt this?

Let me check this, my current logic just lands you on the file where the definition is stored I believe. Its a bit short sighted but I was just assuming at that time people would be using small imports to check out a sandbox. I can refine this tho

I think it's necessary to bring them to the line/column if that would work. otherwise, finding the import is just as good since the user likely only has a few tabs open in the sandbox

I will have a look at the first one, second one is up and running