Contained document refactoring

[tmat]

Fixes #28322

[tmat]

@jasonmalinowski @KirillOsenkov Still work in progress, but wanted to gather initial feedback to see if this is going the right direction.

[KirillOsenkov]

Our goal is for the bulk of this code to land in a non-VisualStudio layer and still work.

For reference here's the MonoDevelop copy of the code:
http://source.monodevelop.com/#MonoDevelop.CSharpBinding/MonoDevelop.Ide.Completion.Presentation/MonoDevelopContainedDocument.cs,fd8c80b4797cb921

I would love to kill it and move to the same shared implementation.

Adding @ToddGrun and @DavidKarlas FYI.

Also my advice: cover it with unit-tests. That logic is gnarly.

[tmat]

Also my advice: cover it with unit-tests. That logic is gnarly.

I'll add some unit tests. I found we had some that were commented out :-/ I do not have understanding what the expected behavior should be though, so it might be better if someone who knows added more tests afterwards.

[KirillOsenkov]

I guess nobody knows this code so you own it now :) The way I'd approach it is put a breakpoint in that code, run a simple ASP.NET Core app, open a .cshtml file, hit Return.

This will trigger the breakpoint. Capture the inputs that go into here and the outputs and that should be enough to give a decent coverage.

[tmat]

@KirillOsenkov I think i'm gonna give up on adding the tests. I can't figure out how to set up the projection buffers :(.

[KirillOsenkov]

No worries, but I thought there are quite a few methods that just deal with strings. Maybe at least try for those?

[KirillOsenkov]

Also I was thinking maybe @ToddGrun or @NTaylorMullen would be interested to help? This is a worthy and important effort.

[NTaylorMullen]

Also I was thinking maybe @ToddGrun or @NTaylorMullen would be interested to help? This is a worthy and important effort.

Definitely sounds worthy! If only I had the bandwidth 😢

[tmat]

If anyone can describe what is the layering of projection buffers and their content types that the VS actually creates, I could use that info to set up the buffers in the tests.

[KirillOsenkov]

LOL the best doc I have is this:

Feel free to stop by for an explanation. We can debug and I can show you the buffers, we can also capture the exact contents and span positions for testing.

[jasonmalinowski]

Comments so far.

[tmat]

Figured out the projection buffer setup.

Now I'm getting an InvalidOperationException from the editor for some line endings:

Also the contained buffer impl ends up adding line breaks that shouldn't be there (I think).
Not sure, what's going on... any ideas?

[KirillOsenkov]

This is precisely why we needed to terraform this code and cover it with tests ;) Thanks for doing this.

Maybe if @NTaylorMullen is busy he can at least help with answering questions, or perhaps @ToddGrun. Or who could we ask?