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

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

Open
mbana opened this issue Dec 15, 2016 · 19 comments
Open

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

mbana opened this issue Dec 15, 2016 · 19 comments
Labels
feature-request Request for new features or functionality typescript typescript-multifile
Milestone

Comments

@mbana
Copy link

mbana commented Dec 15, 2016

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
Copy link
Member

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

@alexdima alexdima added feature-request Request for new features or functionality editor-api editor-core labels Jan 13, 2017
@alexdima alexdima added this to the Backlog milestone Jan 13, 2017
@kitsonk
Copy link

kitsonk commented Jun 17, 2017

@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
Copy link
Member

alexdima commented Jun 23, 2017

@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

@kitsonk
Copy link

kitsonk commented Jun 23, 2017

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

@kitsonk
Copy link

kitsonk commented Jul 7, 2017

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
Copy link

AviVahl commented Nov 12, 2018

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
Copy link
Member

@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
Copy link
Contributor

mofux commented Nov 12, 2018

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
Copy link

AviVahl commented Nov 16, 2018

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:

  • separating monaco's source code into this repo, giving it a TypeScript-like release cycle.
  • refactoring AMD syntax to ESM imports/exports, while still providing AMD/UMD bundles to be consumed by vscode.
  • generating .d.ts files automatically from code, using typescript's built-in abilities.

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
Copy link

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
Copy link

frankLife commented Jan 1, 2019

@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
Copy link

frankLife commented Jan 1, 2019

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
Copy link

ChrisFan commented Jan 3, 2019

@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
Copy link

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

@akosyakov
Copy link

@alexandrudima #291 (comment)
seems to be regression

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

@alexdima
Copy link
Member

I've created #1296 to track the regression on our side...

@blois
Copy link

blois commented Jun 3, 2019

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
Copy link
Member

This might have improved after microsoft/vscode#85129

@Timoniann
Copy link

@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?

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

No branches or pull requests