Skip to content
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

Goto definition (Ctrl + click) works only if definition is in same model #2000

Closed
martin-pabst opened this issue Jun 9, 2020 · 8 comments · Fixed by microsoft/vscode#177064
Labels
feature-request Request for new features or functionality typescript typescript-multifile

Comments

@martin-pabst
Copy link

martin-pabst commented Jun 9, 2020

monaco-editor version: 0.20.0
Browser: Chrome
OS: Windows

Reproduction steps:

  • Go to playground

  • Paste code below

  • Run

  • ctrl + MouseOver on IdentifierA in line 1
    => Identifier A is blue and underlined; small widget pops up showing reference "another IdentifierA" => OK!

  • ctrl left click on IdentifierA in line 1
    => IdentifierA in line 3 blinks and Cursor jumps to line 3 => OK!

  • ctrl + MouseOver on IdentifierB in line 2
    => Identifier B is underlined; small widget pops up showing reference "another IdentifierB" => OK!

  • ctrl left click on IdentifierB in line 2
    => Nothing happens. Editor should display model2 and Curser should jump to line 1 => Bug!

I was able to fix this bug:
a) insert
editor.setModel(model);
in standaloneCodeServiceImpl.ts after line 43
b) replace
return null;
whith
return monaco.editor.getModels().find(model => model.uri.toString() === resource.toString());
in Method StandaloneCodeEditorServiceImpl.findModel

Instead of b) it would certainly be better to use SimpleEditorModelResolverService, but i don't know how to get a reference of it so i better refrain from making a pull request on base of my amateur work.

By the way:
I'm writing an online-IDE with compiler for my students. Without monaco editor this would not be possible:

Monaco editor is an awesome library, thank you!

Editor Playground html:

<div id="container" style="height:100%;"></div>

Editor Playground css:
none

Editor Playground js:

const editor = monaco.editor.create(document.getElementById("container"), {});

const model1 = monaco.editor.createModel("identifierA\nIdentifierb\nanother IdentifierA", "python", monaco.Uri.from({
    scheme: "inmemory",
    path: "file1.macc",
}))

const model2 = monaco.editor.createModel("another Identifierb", "python", monaco.Uri.from({
    scheme: "inmemory",
    path: "file2.macc",
}))

editor.setModel(model1)

monaco.languages.registerDefinitionProvider("python", {
    provideDefinition: (model, position, token) => {
        if (position.lineNumber == 1) {
            return {
                uri: model1.uri,
                range: {
                    startLineNumber: 3,
                    startColumn: 9,
                    endLineNumber: 3,
                    endColumn: 20
                }
            }
        }

        if (position.lineNumber == 2) {
            return {
                uri: model2.uri,
                range: {
                    startLineNumber: 1,
                    startColumn: 9,
                    endLineNumber: 1,
                    endColumn: 20
                }
            }
        }
    }
});
@spahnke
Copy link
Contributor

spahnke commented Jun 25, 2020

I think it would not be a good idea to implement this feature generally; or at least it should be configurable to keep the current behavior. I would assume most use cases use a single editable model (the one that's shown), accompanied by zero or more additional models.

For example: We add several models to the editor (all .js, or .d.ts files) to provide code completion for complex scenarios, but would never ever ever want a user to navigate there in the same editor instance by invoking "Go to definition". Because that would mean we are now in a state that is absolutely meaningless to the user, and additionally there is no way for the user to get back to the old model.

However, I see a need to provide functionality like this - even in our use case. We would like for the user to "Go to definition" for specific models. Not in the same editor instance, but rather in a new tab. Today we accomplish that by hijacking the openEditor function on the StandaloneCodeEditorServiceImpl class. If the original function returns null we just check if it is a valid model to jump to, and then open a new tab displaying that code, but leaving the original editor in the same state.

Playground code:

monaco.editor.createModel("const foo = 1;", "javascript", monaco.Uri.file("foo.js"));

const editor = monaco.editor.create(document.getElementById("container"), {
    value: `function hello() {
	alert('Hello world!');
}
hello();
foo`,
    language: "javascript"
});

const editorService = editor._codeEditorService;
const openEditorBase = editorService.openCodeEditor.bind(editorService);
editorService.openCodeEditor = async (input, source) => {
    const result = await openEditorBase(input, source);
    if (result === null) {
        alert("intercepted")
        console.log("Open definition for:", input);
        console.log("Corresponding model:", monaco.editor.getModel(input.resource));
    }
    return result; // always return the base result
};

By adjusting the code a bit I would assume you could use the same technique to open the found editor model in the same editor instance (the source parameter is a ICodeEditor instance).

The downside is that this is not an official API. So the question is could there be an official API endpoint that enables registering some kind of provider/handler that kicks in when the standard implementation does not find a model (i.e. the if block in my hacky implementation)? If we settle on an approach and I find some time, I would be happy to provide a PR.

Thoughts?

@martin-pabst
Copy link
Author

Dear Mr. Pahnke,
thank you for your proposal! I'm able to achieve the desired functionality with this code:

const editorService = editor._codeEditorService;
const openEditorBase = editorService.openCodeEditor.bind(editorService);
editorService.openCodeEditor = async (input, source) => {
    const result = await openEditorBase(input, source);
    if (result === null) {
        alert("intercepted")
        console.log("Open definition for:", input);
        console.log("Corresponding model:", monaco.editor.getModel(input.resource));
        console.log("Source: ", source);
        source.setModel(monaco.editor.getModel(input.resource));
    }
    return result; // always return the base result
};

So an official API endpoint that enables registering some kind of provider/handler that kicks in when the standard implementation does not find a model would be a good solution.

@ivan-prodanov
Copy link

@alexdima can we have a Single(default) and Multifile option on the editor options to mitigate this issue? This way the editor will stay compatible with current use cases but also widely open the doors to VSCode-like browser editors, which support multiple files.

@geekroscom
Copy link

How to switch to Python language, it seems that it can't be used

@samtheeagle578
Copy link

samtheeagle578 commented Feb 8, 2023

Hello @spahnke ,

Has the implementation of the editor changed recently in a way that your solution above stopped working? I even get an error running it in the playground.

Any help is much appreciated.

Kind Regards,
Sam

@spahnke
Copy link
Contributor

spahnke commented Feb 8, 2023

I believe since version 0.35.x the Monaco project started to minify/mangle every internal API, so that it has become impossible to hook into any internal objects/methods. I don't have a way forward myself yet 😕

@samtheeagle578
Copy link

samtheeagle578 commented Feb 9, 2023

Thank you for your help. I was able to get it working using your example. The following threads also helped a lot: #2407 #291 (comment)

@hediet
Copy link
Member

hediet commented Mar 13, 2023

I believe since version 0.35.x the Monaco project started to minify/mangle every internal API

This will hopefully be fixed soon for the monaco editor.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality typescript typescript-multifile
Projects
None yet
7 participants