Skip to content

Ensure that SignatureHelp and SignatureHelp has non-null arrays - #112

Merged
akosyakov merged 2 commits into
TypeFox:masterfrom
rcjsuen:signatureHelp
Sep 3, 2018
Merged

Ensure that SignatureHelp and SignatureHelp has non-null arrays#112
akosyakov merged 2 commits into
TypeFox:masterfrom
rcjsuen:signatureHelp

Conversation

@rcjsuen

@rcjsuen rcjsuen commented Sep 1, 2018

Copy link
Copy Markdown
Contributor

While the LSP allows a SignatureHelp's signatures array to be optional, Monaco does not so we should ensure that it Monaco's representation of a SignatureHelp is properly initialized with a zero length array if the value from the language server is null or undefined.

See microsoft/monaco-editor#1045.

While the LSP allows a SignatureHelp's signatures array to be
optional, Monaco does not so we should ensure that it is
initialized with a zero length array if the value from the language
server is null or undefined.

Signed-off-by: Remy Suen <remy.suen@gmail.com>
While the LSP allows a SignatureInformation's parameters array to be
optional, Monaco does not so we should ensure that it is initialized
with a zero length array if the value from the language server is null
or undefined.

Signed-off-by: Remy Suen <remy.suen@gmail.com>
@rcjsuen

rcjsuen commented Sep 1, 2018

Copy link
Copy Markdown
Contributor Author

The parameters array in SignatureInformation is actually the culprit of the bug in microsoft/monaco-editor#1045.

Either way, we should fix both of these cases...

@rcjsuen rcjsuen changed the title Ensure that a SignatureHelp has a non-null array Ensure that SignatureHelp and SignatureHelp has non-null arrays Sep 1, 2018

@akosyakov akosyakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@rcjsuen thank you!

@akosyakov
akosyakov merged commit 3f0a16a into TypeFox:master Sep 3, 2018
@rcjsuen
rcjsuen deleted the signatureHelp branch September 3, 2018 10:48
@hifall

hifall commented Sep 5, 2018

Copy link
Copy Markdown

Is this PR supposed to fix Monaco Editor #1045? I have tried to upgrade to 0.8.0, but the crash issue still persists.

@rcjsuen

rcjsuen commented Sep 5, 2018

Copy link
Copy Markdown
Contributor Author

Hi, @hifall. It should but it sounds like I missed something.

Please open a bug and describe your setup and versions there.

@hifall

hifall commented Sep 5, 2018

Copy link
Copy Markdown

Filed as Issue #115.

@rcjsuen rcjsuen mentioned this pull request Sep 5, 2018
@hifall

hifall commented Sep 17, 2018

Copy link
Copy Markdown

Confirmed working. Thanks!

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.

3 participants