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.

Sign up for GitHub

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

Jump to bottom

Is it expected that in 1.16, Hovers with null items in their arrays do not render at all #34520

Closed
DanTup opened this issue Sep 16, 2017 · 5 comments
Closed

Is it expected that in 1.16, Hovers with null items in their arrays do not render at all #34520

DanTup opened this issue Sep 16, 2017 · 5 comments
Assignees
@jrieken
Labels
bug Issue identified by VS Code Team member as probable bug editor-contrib Editor collection of extras verified Verification succeeded
Milestone
September 2017

Comments

@DanTup
Copy link
Contributor

DanTup commented Sep 16, 2017

This seems to be a breaking change in 1.16. I don't know if it's a bug or the old behaviour is considered bad, but it's resulted in a lot of hovers no longer rendering after the update.

HoverProvider contains Code looks like this:

resolve(new Hover(
	[{ language: 'dart', value: data.displayString }, data.documentation],
	range
));

The data passed to the hover is an array - the first item is an object with a language/value and the second item is additional documentation (which is just a markdown string).

If data.documentation is null in 1.15, the tooltip still rendered, just ignoring the second item:

Working hovers

However after updating, the hover does not appear at all when there's a null in that array.

Broken hovers

Is this an expected change? I can update my extension not to put nulls in here, though I don't know whether this might affect other extensions too. It was kinda convenient for Code to skip over this nulls, we could just string a bunch of variables together that may or may not be set for a given hover, but now we'll need to conditionally push them onto an array before sending.
@vscodebot vscodebot bot added the debug Debug viewlet, configurations, breakpoints, adapter issues label Sep 16, 2017
@DanTup DanTup mentioned this issue Sep 16, 2017
Closed
@DanTup
Copy link
Contributor Author

DanTup commented Sep 16, 2017

This only seems to happen for nulls... If I do data.documentation || undefined it appears fine.

@isidorn isidorn removed the debug Debug viewlet, configurations, breakpoints, adapter issues label Sep 18, 2017
@isidorn isidorn assigned jrieken and unassigned isidorn Sep 18, 2017
@jrieken jrieken added this to the September 2017 milestone Sep 18, 2017
@jrieken jrieken added bug Issue identified by VS Code Team member as probable bug editor-contrib Editor collection of extras labels Sep 18, 2017
@jrieken
Copy link
Member

jrieken commented Sep 18, 2017

Will investigate...

@jrieken
Copy link
Member

jrieken commented Sep 20, 2017

Yep, shows this error for me

shell.js:228 Cannot read property 'language' of null: TypeError: Cannot read property 'language' of null
	at isCodeblock (/Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostTypeConverters.js:132:32)
	at from (/Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostTypeConverters.js:136:17)
	at Array.map (native)
	at Object.fromMany (/Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostTypeConverters.js:127:27)
	at Object.fromHover (/Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostTypeConverters.js:261:38)
	at /Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostLanguageFeatures.js:155:45
	at Promise_ctor.CompletePromise_then [as then] (/Users/jrieken/Code/vscode/out/vs/base/common/winjs.base.raw.js:1566:49)
	at HoverAdapter.provideHover (/Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostLanguageFeatures.js:145:119)
	at /Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostLanguageFeatures.js:701:96
	at ExtHostLanguageFeatures._withAdapter (/Users/jrieken/Code/vscode/out/vs/workbench/api/node/extHostLanguageFeatures.js:629:20)

@jrieken
Copy link
Member

jrieken commented Sep 20, 2017

Always refreshing to get remained about the fact that typeof null === 'object'....

@DanTup
Copy link
Contributor Author

DanTup commented Sep 20, 2017

Great, thanks! 👍

@deysaikat95 deysaikat95 mentioned this issue Sep 25, 2017
Closed
@roblourens roblourens added the verified Verification succeeded label Sep 28, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jrieken jrieken

Labels
bug Issue identified by VS Code Team member as probable bug editor-contrib Editor collection of extras verified Verification succeeded
Projects
None yet
Milestone
September 2017
Development

Successfully merging a pull request may close this issue.

Update from original (#1)
4 participants
@roblourens @DanTup @jrieken @isidorn