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

Peek reference and preview definition don't work across models #935

Closed
masad-frost opened this issue Jun 28, 2018 · 5 comments · Fixed by microsoft/vscode#85129
Closed
Labels
feature-request Request for new features or functionality

Comments

@masad-frost
Copy link

Reproduction steps:

  • Go to playground
  • Paste code below
  • Run
  • Right click on created editor
  • Peek reference
  • Works fine for same model
  • Throws an error if you try to look at reference in another model

Same thing happens with peek definition with a different error.

Seems like the model resolution is failing.

Example simple code:

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

const model = monaco.editor.createModel("a", null, monaco.Uri.from({
    scheme: "inmemory",
    path: "file1.py",
}))

const model2 = monaco.editor.createModel("b", null, monaco.Uri.from({
    scheme: "inmemory",
    path: "file2.py",
}))

editor.setModel(model)

monaco.languages.registerReferenceProvider("python", {
    provideReferences: (model, position, context, token) => {
        return [
            {
                uri: model.uri,
                range: {
                    startLineNumber: 1,
                    startColumn: 1,
                    endLineNumber: 1,
                    endColumn: 2
                }
            },
            {
                uri: model2.uri,
                range: {
                    startLineNumber: 1,
                    startColumn: 1,
                    endLineNumber: 1,
                    endColumn: 2
                }
            }]
    }
})
@alexdima alexdima added the feature-request Request for new features or functionality label Jun 29, 2018
@masad-frost
Copy link
Author

masad-frost commented Jul 3, 2018

So I was able to get this to work by overriding textModelService (I wish there's a list of overrides somewhere)

const editor = monaco.editor.create(
  document.getElementById("container"),
  {},
  {
    textModelService: {
      createModelReference: resource => {
        const resourceModel = monaco.editor.getModel(resource);
        const simpleModel = resourceModel
          ? new SimpleModel(resourceModel)
          : null;

        return window.monaco.Promise.as({
          object: simpleModel,
          dispose: () => {}
        });
      },
      registerTextModelContentProvider: () => ({ dispose: () => {} })
    }
  }
);

@alexandrudima does this look ok to you?

@alexdima
Copy link
Member

alexdima commented Jul 3, 2018

@masad-frost
Copy link
Author

I'll keep an eye out for changes. Thanks for the list

By the way we adopted Monaco at repl.it and are using language servers https://repl.it/site/blog/intel. It's a really cool and transformative project, thank you for maintaining it.

@ulrichb
Copy link
Contributor

ulrichb commented Jul 30, 2018

Note that also "Go to definition" doesn't work across models by default. Would also be nice to work out of the box. See #852 for a workaround.

EDIT: Also hitting Enter in Peek definition needs the workaround in #852 (a defined editorService / openEditor).

@abhaygupta
Copy link

abhaygupta commented Nov 21, 2018

Was able to get peek, find all references and goto definition work across multiple models/tabs - had to override textModelService (as discussed earlier). This is how code change while creating monaco editor instance looks like for me ..

const editor = monaco.editor.create(document.getElementById("container")!, {
    glyphMargin: true,
    lightbulb: {
        enabled: true
    },
    }, { 
        editorService: tkeLangCodeEditorService,
        textModelService: {
                createModelReference: function(uri: monaco.Uri) {
                    const textEditorModel = {
                        load() {
                        return Promise.resolve(textEditorModel)
                        },
                        dispose() {},
                        textEditorModel: monaco.editor.getModel(uri)
                    }
                    return Promise.resolve({
                        object: textEditorModel,
                        dispose() {}
                    })
                },
                registerTextModelContentProvider: () => ({ dispose: () => {} })
        }
});

to make goto definition work across multiple models/tabs, had to override editorService - findModel and doOpenEditor methods. As these function are defined to work in single editor / tab environment ..

Existing standalone editorService implementation - with URI check in findModel:

StandaloneCodeEditorServiceImpl.prototype.findModel = function (editor, resource) {
	    var model = editor.getModel();
        if (model.uri.toString() !== resource.toString()) {
           return null;
        }
        return model;
    };

Enhancement to make it work for multiple models/tab:

StandaloneCodeEditorServiceImpl.prototype.findModel = function (editor, resource) {
    var model = null;
    if(resource !== null)
        model = monaco.editor.getModel(resource);
    if(model == null) {
        model = editor.getModel()
    }
    return model;
};

StandaloneCodeEditorServiceImpl.prototype.doOpenEditor = function (editor, input) {
    var model = this.findModel(editor, input.resource);
    //set editor to new model
    if(model)
        editor.setModel(model);
        
    if (!model) {
        if (input.resource) {
            var schema = input.resource.scheme;
            if (schema === Schemas.http || schema === Schemas.https) {
                // This is a fully qualified http or https URL
                windowOpenNoOpener(input.resource.toString());
                return editor;
            }
        }
        return null;
    }
    var selection = input.options.selection;
    if (selection) {
        if (typeof selection.endLineNumber === 'number' && typeof selection.endColumn === 'number') {
            editor.setSelection(selection);
            editor.revealRangeInCenter(selection, 1 /* Immediate */);
        }
        else {
            var pos = {
                lineNumber: selection.startLineNumber,
                column: selection.startColumn
            };
            editor.setPosition(pos);
            editor.revealPositionInCenter(pos, 1 /* Immediate */);
        }
    }
    return 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants