microsoft / monaco-editor

A browser based code editor
https://microsoft.github.io/monaco-editor/
MIT License
40.45k stars 3.6k forks source link

go to definition - different model: loading via http #291

Open mbana opened 7 years ago

mbana commented 7 years ago

i'm adding a go to definition feature using a langserver.

monaco notices it's a new model πŸ‘, and then tries to load it. the ranges of the place that contains definition is correct.

does monaco support in-place replacement of the model? what i'm seeing is that it just offers to download the whole file containing the resulting definition, i.e., this line window.open(data.resource.toString());.


https://github.com/Microsoft/vscode/blob/0d6a9f8389cd044851ac994e527969bd53d2d387/src/vs/editor/browser/standalone/simpleServices.ts#L134

in my monaco build:

private doOpenEditor(editor:editorCommon.ICommonCodeEditor, data:IResourceInput): IEditor {
    var model = this.findModel(editor, data);
    if (!model) {
        if (data.resource) {
            if (this.openEditorDelegate) {
                this.openEditorDelegate(data.resource.toString());
                return null;
            } else {
                var schema = data.resource.scheme;
                if (schema === Schemas.http || schema === Schemas.https) {
                    // This is a fully qualified http or https URL
                    window.open(data.resource.toString());
                    return this.editor;
                }
            }
        }
        return null;
    }

    var selection = <editorCommon.IRange>data.options.selection;
    if (selection) {
        if (typeof selection.endLineNumber === 'number' && typeof selection.endColumn === 'number') {
            editor.setSelection(selection);
            editor.revealRangeInCenter(selection);
        } else {
            var pos = {
                lineNumber: selection.startLineNumber,
                column: selection.startColumn
            };
            editor.setPosition(pos);
            editor.revealPositionInCenter(pos);
        }
    }

    return this.editor;
}
alexdima commented 7 years ago

You would need to provide a custom implementation of IEditorService. Services can be overriden when creating the editor (last argument):

    /**
     * Create a new editor under `domElement`.
     * `domElement` should be empty (not contain other dom nodes).
     * The editor will read the size of `domElement`.
     */
    export function create(domElement: HTMLElement, options?: IEditorConstructionOptions, override?: IEditorOverrideServices): IStandaloneCodeEditor;

i.e.

monaco.editor.create(domNode, options, { editorService: myEditorServiceInstanceHere });

You would need to find the service shape from vscode's sources, we do not plan to expose all services shapes in the public API (monaco.d.ts). You can also consult the simple services implementation we ship with simpleServices.ts

kitsonk commented 7 years ago

@alexandrudima I have tried doing as instructed, but I am not getting the editor service called when I would expect it to...

Here is the stub service I have at the moment:

class EditorService {
    _serviceBrand: any;
    openEditor(input: ResourceInput, sideBySide?: boolean): Promise<monaco.editor.IEditor> {
        console.log('openEditor', input);
        return Promise.resolve({});
    }
    resolveEditor(input: ResourceInput, refresh?: boolean): Promise<TextEditorModel> {
        console.log('resolveEditor', input);
        return Promise.resolve({});
    }
}

And here is how I am creating my editor instance:

const editorService = new EditorService();
const editor = monaco.editor.create(_root, options, { editorService });

But I can't seem to find any action in the editor that calls those methods. I have tried the "Go to Definition" and the other commands and I am just not getting anything logged to the console. It feels like there is something else I need to wire up...

gotodef

alexdima commented 7 years ago

@kitsonk :+1: That is the correct way to inject a different editorService in the editor. Here is a complete self-contained example that triggers the editorService:

You can run it anywhere in the playground


monaco.languages.register({ id: 'mySpecialLanguage' });

monaco.languages.registerDefinitionProvider('mySpecialLanguage', {
    provideDefinition: function(model, position, cancellationToken) {
        return {
            uri: monaco.Uri.parse('http://a/different/file.txt'),
            range: new monaco.Range(3, 1, 3, 1)
        };
    }
});

var editorService = {
    openEditor: function() {
        alert(`open editor called!` + JSON.stringify(arguments));
    },
    resolveEditor: function() {
        alert(`resolve editor called!` + JSON.stringify(arguments));
    }
};

monaco.editor.create(document.getElementById("container"), {
    value: '\n\Go to definition on this text',
    language: 'mySpecialLanguage'
}, { editorService: editorService });

editor-291

kitsonk commented 7 years ago

Ok, thanks!! I will try again and try to figure where I got it wrong. 😁

kitsonk commented 7 years ago

Just for future generations... I was missing a definition provider for the language I was trying to provide the editor service for. Without a definition provider, the editor service doesn't get called.

AviVahl commented 6 years ago

You would need to find the service shape from vscode's sources, we do not plan to expose all services shapes in the public API (monaco.d.ts).

I find this statement a bit counter productive and am asking the monaco/vscode team to reconsider.

I am writing a custom integration to monaco where I have to replace some of these services as well. Instead of enjoying the fact that vscode/monaco are written using TypeScript, I find myself having to go to original sources and copy-paste interfaces to get a properly typed integration.

The types are there, and the implementation can be overridden using the public API (third parameter of constructor). Is there any con to exposing all services interfaces as well?

alexdima commented 6 years ago

@AviVahl Exposing the services will leak hundreds of new types to monaco.d.ts. It would require quite a lot of work in monaco.d.ts.recipe to accomodate all those types that need to be added. The types in monaco.d.ts do not reflect the reality of the code. The code is written with AMD, while monaco.d.ts uses namespaces. It would also make a lot of the changes my teammates do in vscode to result in changes to monaco.d.ts, which is not something the team appreciates.

The 3rd argument is there exactly to allow custom more advanced integrators to do fun things... If you are doing such custom advanced stuff, I hope you can spend the extra few minutes to copy-paste the services types from the vscode repo and into your project... I know it is not optimal, but please keep in mind that the Monaco Editor is a by-product of VS Code and its sole reason for existance is VS Code.

mofux commented 6 years ago

and its sole reason for existance is VS Code.

I'd argue that that's not completely true anymore. MS uses the monaco-editor outside of VSCode in quite a few places, like in their Azure Cloud products, Microsoft Teams and SQL Operations Studio. But also outside of MS it plays a big role, for example the next-gen IDE of Eclipse (Eclipse Theia) uses the monaco-editor at the heart of their product. The list of dependants keep growing, which proves what a great editor you've created, but it also puts the weight of their expectations on your shoulders.

But rest assured, IMO you're handling this responsibility really well by allowing consumers to extend almost every corner of the editor. Naturally, there are always compromises to be made because otherwise it would stall VSCode development.

AviVahl commented 5 years ago

A type is not "leaking" if it's already part of the public API. When an API is exposed as any, it only means that a user that API will not be aware of signature incompatibilities during build time.

The overhead of generating the .d.ts by a script can be reduced by utilizing a more "conforming" project setup, where typescript auto-generates the .d.ts files from sources (declaration: true).

Copying and inlining interfaces is not a good practice. It's not just the wasted development time. These interfaces can give me a false positive build if I fail to keep them up-to-date with monaco/vscode. In addition, it's not always clear which interface matches which version (mix and match game from my end).

Looking at the growing number of consumers of monaco, I propose a much wider change:

I would (personally) even go as further as creating a monorepo that contains all Microsoft's monaco integrations, along side the editor-core package, with the editor package consuming the editor-core package and adding the integrations.

EDIT: and just in case it wasn't clear... I only suggest these changes because I find your team's work awesome and would like to enjoy using it in more of my own and work projects.

abhaygupta commented 5 years ago

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;
};

Let me know if you guys see any issues with this implementation?

frankLife commented 5 years ago

@alexandrudima I found the demo you provided didn't work. It won't alert and only open a broswer tab page

monaco.languages.register({ id: 'mySpecialLanguage' });

monaco.languages.registerDefinitionProvider('mySpecialLanguage', {
    provideDefinition: function(model, position, cancellationToken) {
        return {
            uri: monaco.Uri.parse('http://a/different/file.txt'),
            range: new monaco.Range(3, 1, 3, 1)
        };
    }
});

var editorService = {
    openEditor: function() {
        alert(`open editor called!` + JSON.stringify(arguments));
    },
    resolveEditor: function() {
        alert(`resolve editor called!` + JSON.stringify(arguments));
    }
};

monaco.editor.create(document.getElementById("container"), {
    value: '\n\Go to definition on this text',
    language: 'mySpecialLanguage'
}, { editorService: editorService });

any update about this demo?

frankLife commented 5 years ago

I find the override params initialization may be rewrited by default option .

in monaco-editor/esm/vs/editor/standalone/browser/standalone/browser/standaloneEditor.js: image

_all.forEach(function (service) { return result.set(service.id, service.get(overrides)); }); may override the passed param

So even though I pass a codeEditorService , I also find the codeEditorService of the editor I create is default implementation of StandaloneCodeEditorServiceImpl which is located in monaco-editor/esm/vs/editor/standalone/browser/StandaloneCodeEditorServiceImpl.js

image

image

the console of chrome broswer always log the StandaloneCodeEditorServiceImpl

@alexandrudima may you can give us a new demo ?

ChrisFan commented 5 years ago

@frankLife as @abhaygupta mentioned, you could just override StandaloneCodeEditorServiceImpl.prototype.doOpenEditor, it works for me.

Still, it would be nice if there's a better solution without override the prototype like @alexandrudima offered

frankLife commented 5 years ago

@ChrisFan Thanks a lot and I will try in this weekend. Maybe we both also need a official way to reach the goal.

akosyakov commented 5 years ago

@alexandrudima https://github.com/Microsoft/monaco-editor/issues/291#issuecomment-450709332 seems to be regression

I've observed the same with Monaco >= 0.14.x, used to work before.

alexdima commented 5 years ago

I've created https://github.com/Microsoft/monaco-editor/issues/1296 to track the regression on our side...

blois commented 5 years ago

It seems like there's quite a bit of re-implementation which is necessary to override the text model and code editor services. And alternate approach which accesses quite a bit more private state but may be more maintainable in the longer approach is to patch the existing services:

const services = [...editor._instantiationService._parent._services._entries.values()];
const textModelService = services.find((service) => 'createModelReference' in Object.getPrototypeOf(service));

// GoToDefinition validates that the range is within the bounds of
// the text model, so just generate a really big one that will work
// for any range.
const navigationTextModel = monaco.editor.createModel(new Array(10000).fill('').join('\n'));

textModelService.createModelReference = async (uri) => {
  const reference = {
    async load() {
      return navigationTextModel;
    },
    dispose() {},
    textEditorModel: navigationTextModel,
  };
  return {
    object: reference,
    dispose() {},
  };
};

const codeEditorService = editor._codeEditorService;
codeEditorService.openCodeEditor = ({resource, options}) => {
  const file = resource.path;
  const range = options.selection;
  // Add code here to open the code editor.
}
alexdima commented 4 years ago

This might have improved after https://github.com/microsoft/vscode/pull/85129

Timoniann commented 3 years ago

@kitsonk πŸ‘ That is the correct way to inject a different editorService in the editor. Here is a complete self-contained example that triggers the editorService:

You can run it anywhere in the playground

monaco.languages.register({ id: 'mySpecialLanguage' });

monaco.languages.registerDefinitionProvider('mySpecialLanguage', {
    provideDefinition: function(model, position, cancellationToken) {
        return {
            uri: monaco.Uri.parse('http://a/different/file.txt'),
            range: new monaco.Range(3, 1, 3, 1)
        };
    }
});

var editorService = {
  openEditor: function() {
      alert(`open editor called!` + JSON.stringify(arguments));
  },
  resolveEditor: function() {
      alert(`resolve editor called!` + JSON.stringify(arguments));
  }
};

monaco.editor.create(document.getElementById("container"), {
  value: '\n\Go to definition on this text',
  language: 'mySpecialLanguage'
}, { editorService: editorService });

editor-291

Hi. Today, your example is not working image image May you provide a new solution?