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

When the language server doesn't provide a resolve option, return the original codelens #46

Merged
merged 1 commit into from
Jan 31, 2018

Conversation

Twinside
Copy link

In browser test shown that monaco still call the resolver in all cases,
so we can't use undefined here.

Thanks for the library :)

…original code lens

In browser test shown that monaco still call the resolver in all cases,
so we can't use undefined.
@@ -271,7 +271,7 @@ export class MonacoLanguages implements Languages {
}
const protocolCodeLens = this.m2p.asCodeLens(codeLens);
return provider.resolveCodeLens!(protocolCodeLens, token).then(result => this.p2m.asCodeLens(result))
} : undefined
} : ((m, codeLens, t) => codeLens)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that it is necessary? resolveCodeLens is optional, I assume monaco does it automatically

Copy link
Author

@Twinside Twinside Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like their is a bug on their side, but I'm not optimistic about the turnaround time, I'll look after making a pull request for them too, from the compiled editor.main.js (line 90163):

            var promises = toResolve.map(function (request, i) {
                var resolvedSymbols = new Array(request.length);
                var promises = request.map(function (request, i) {
                    return async_1.asWinJsPromise(function (token) {
                        return request.provider.resolveCodeLens(model, request.symbol, token);
                    }).then(function (symbol) {
                        resolvedSymbols[i] = symbol;
                    });
                });
                return winjs_base_1.TPromise.join(promises).then(function () {
                    lenses[i].updateCommands(resolvedSymbols);
                });
            });

No care taken for the optionsal resolveCodeLens

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you are right, please file an issue for monaco-editor as well

@akosyakov akosyakov merged commit f0f2c10 into TypeFox:master Jan 31, 2018
@akosyakov
Copy link
Contributor

@Twinside thank you

@Twinside
Copy link
Author

Oh, you already filled a ticket: microsoft/monaco-editor#576

@Twinside Twinside deleted the for-pr branch March 15, 2018 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants