Error updating diagnostics from extension

[bartosz-antosik]

There is some strange behavior of diagnosticsCollection.set() in recent builds.

I have created and maintain spell checking extension Spell Right it has recently had a strange behavior reported #213. It seems that there is some problem with:

this.diagnosticCollection.set(document.uri, diagnostics);

It looks like it does not update diagnostics!

Steps to Reproduce:

1. Start VSCode with Spell Right
2. Try to type something, after typing space at the end of misspelled word misspelled word should get underlined. It does not, but strangely enough Ctrl+. does provide spelling suggestions as you can see in the screenshot below:

Summing up it does not update diagnostics in the panel neither does it display the underlines for the diagnostics but somehow the menu with diagnostics resolutions' suggestions does work in correct places!

The code stopped working as it was reported around the time above mentioned issue #213 was reported. I am aware that there were some changes around the diagnostics functionality lately which lead to few reports like #58564. I am unable however to draw any conclusion about what am I possibly doing wrong.

Version information:

Version: 1.28.0 (system setup)
Commit: 431ef9d
Date: 2018-10-05T14:58:53.203Z
Electron: 2.0.9
Chrome: 61.0.3163.100
Node.js: 8.9.3
V8: 6.1.534.41
Architecture: x64
Windows 10 Pro

Same acting in Insiders version:

Version: 1.29.0-insider (system setup)
Commit: 5bd6ebe
Date: 2018-10-09T09:31:07.090Z
Electron: 2.0.11
Chrome: 61.0.3163.100
Node.js: 8.9.3
V8: 6.1.534.41
Architecture: x64

[vscodebot]

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

• Spelling errors (diagnostics) prevent debugging (#29730)

[PeteX]

I'm getting symptoms that appear similar to bartosz-antosik/vscode-spellright#218 but I'm on the release build of Code, on Linux:

Version: 1.28.0
Commit: 431ef9d
Date: 2018-10-05T14:52:58.945Z
Electron: 2.0.9
Chrome: 61.0.3163.100
Node.js: 8.9.3
V8: 6.1.534.41
Architecture: x64

It's possible it's a different problem but I thought I'd flag up the possibility that it's occurring on this release too.

[ahmadawais]

I have the same issue and it's quite cumbersome to deal with

1.28.1
3368db6750222d319c851f6d90eb619d886e08f5
x64

[jrieken]

Unsure, the only recent change was that we don't send update the renderer/main thread when diagnostics appear as equal to be 'before' set of diagnostics. @bartosz-antosik Where do you create your diagnostic objects, do you use vscode.Diagnostic or another type? Can you boil this down to a small extension that reproduces this?

[bartosz-antosik]

@jrieken Thank you for finally noticing the issue.

I use what is returned by vscode.languages.createDiagnosticCollection('spellright') and I believe this is DiagnosticCollection. It is created once per extension's life and updated using said this.diagnosticCollection.set(document.uri, diagnostics) at various points.

Correct me, but what seems pretty obvious is that your comparison is not working!

As you can see in the above post diagnostics DO get updated when they are NOT displayed (e.g. screenshot)!

What may be important is that my extension crafts diagnostics quite carefully. Spell Right does use editing notifications to spell only very little things around what has been changed. And this requires me to manage diagnostics e.g. only delete what has been changed and insert diagnostics in order (e.g. between already existing diagnostics). I do not clean diagnostics and stuff them with everything using push but I delete & insert items. So my updates may bring very subtle changes.

I really do not think there is too much sense (apart from that it would be very problematic to me in terms of free time to do this) in preparing a special extension because you may take Spell Right, start with empty text document, type some nonsense word (please remember that it will not push diagnostics for the word being typed, but rather after first following white character/separator), and see what it sets as diagnostics few times from inside of VSCode and it absolutely should show you what is wrong with comparison.

[bartosz-antosik]

@jrieken I think I should add this: diagnostics items are mutated: that is they contain fields important to Spell Right. Do not know whether this has any influence on the comparison process. It worked just fine with many previous editions of VSCode.

[jrieken]

@jrieken Thank you for finally noticing the issue.

Oh, I am very sorry that we go on vacation from time to time

@bartosz-antosik Where do you create your diagnostic objects, do you use vscode.Diagnostic or another type?

Please answer this question. Thanks

[bartosz-antosik]

@jrieken Thank you for finally noticing the issue.

Oh, I am very sorry that we go on vacation from time to time

Terribly sorry! Just did not expect issue can be assigned to someone on vacation. Seriously I am bit bitter for which I am truly sorry but I am quite sure I have not changed anything and the extension which few people use got blown up.

@bartosz-antosik Where do you create your diagnostic objects, do you use vscode.Diagnostic or another type?

Please answer this question. Thanks

This is how I create diagnostic (quote from code):

var diag = new vscode.Diagnostic(range, message, diagnosticsType);
diag.source = 'spelling';

Is it possible at all to use any other type there?

[PeteX]

I'm just pleased there is a great editor, with great extensions, that I can use for free. So please don't kill each other, who would maintain the editor then? :-)

[bartosz-antosik]

I apologize. I was bit stressed because basically the extension stopped to work. I have done extensive investigation trying to tie scarce threads about the changes that were done to the diagnostics mechanism & adjust the code according to my suspicions, but nothing seemed to work which lead to the issue above. In other words: although I have put quite some work there I felt helpless.

[PeteX]

I meant what I said about appreciating the extension, by the way. You fixed my bug (unrelated to this one) very quickly, even though I think it's only a spare time project for you. Thank you.

[bartosz-antosik]

Thank you for your appreciation. Yes, this is a spare time project indeed. Started out of void between not working and IMHO overshoot solutions (#20266).

[jrieken]

no worries @bartosz-antosik. the day after vacation is always the most stressful... I will take a look this tomorrow.

[jrieken]

This is a bug on our end. We have added logic that prevents unnecessary IPC messages between the renderer and the extension when a diagnostics change actually don't change anything (at least nothing that's rendered). The implementation compares the existing diagnostics array with the new one and won't send an update when they are equal.

Bug is the that array isn't copied before storing but kept as-is. Now, SpellRight rightly reuses the array that it passes to the DiagnosticsCollection#set(uri, diagnostics) call, making us compare the array against itself, us not forwarding the update...

@bartosz-antosik I will push a fix for insiders. An easy, temporary, fix/workaround on your side would be to slice(0) the array before calling set