[FUSE] Fix C# completion in empty explicit expression blocks #11282
Conversation
| @@ -53,6 +53,14 @@ protected override void BuildRenderTree(global::Microsoft.AspNetCore.Components. | |||
| #line hidden | |||
| #nullable disable | |||
| ); | |||
| #nullable restore | |||
| #line (3,23)-(4,1) "x:\dir\subdir\Test\TestComponent.cshtml" | |||
There was a problem hiding this comment.
Should we avoid emitting #line pragmas for whitespace nodes? (We only need source mappings right?)
There was a problem hiding this comment.
I'll let @chsienki comment on this, because he recently made it so that line pragmas and source mappings were 1:1 in runtime. Obviously at that point whitespace wouldn't have been part of it, but I'm not sure which priority wins.
There was a problem hiding this comment.
My original assumption here was that at some point in the future, we may not have any source mappings: if we're getting the docs from the generator, we might not have the back channel for the mappings.
I think with the current architecture that's actually not going to be true, but I do think its a good idea to maintain a 1-1 mapping, so the source doc contains all the information (especially for some of our strategic partners who are using the line pragmas only)
| var content = codeDocument.Source.Text.GetSubText(new TextSpan(sourceMapping.OriginalSpan.AbsoluteIndex, sourceMapping.OriginalSpan.Length)).ToString(); | ||
| if (string.IsNullOrWhiteSpace(content)) | ||
| { | ||
| continue; |
There was a problem hiding this comment.
It seems there are sometimes #line pragmas emitted for whitespace. Presumably that doesn't happen always so we need to skip the validation - is there a reason it happens sometimes and not always?
There was a problem hiding this comment.
My thinking was thus: The existing design time code-gen sometimes doesn't emit line pragmas, but none of the existing tests happened to cover that scenario. I decided to update the test validation here (it now matches the runtime validation, just below this change) rather that updating design time to emit line pragmas for whitespace (which runtime does) largely because updating design time code gen seems quite redundant at this point.
Could be overruled by an actual compiler team member, or maybe runtime shouldn't emit line pragmas either (see my reply to your other comment though), but I don't really have an opinion about which is best.
There was a problem hiding this comment.
Not sure I understand
I decided to update the test validation here (it now matches the runtime validation, just below this change)
Because AFICT the only change is to skip the whitespace checks.
My assumption is this wasn't failing because none of the previous tests hit it, but the test you added does now cause this validation to fail, unless you skip the whitespace?
I'm fine with this change, but we also have an escape hatch to just not run verification on design time if it's preferred to do that.
There was a problem hiding this comment.
Yeah, that's what I meant. I updated the validation done by tests, to skip validating line pragmas on whitespace.
Fixes #11280
Commit at a time is recommended, since the fix itself is really very simple, and the last commit is updating all of the baselines.