Makes mouse/left/right navigation skip injected text. Also fixes bracket decoration bug.

[hediet]

Please don't review yet, there is a bug. Fixed.

[alexdima]

Looks good, but very complex! Took me quite a while to understand.

I've pushed the following changes directly:

• avoid allocating a Map and strings to set as keys every time the view state is set, especially since 90% or more of the time the calls to viewModel.normalizePosition will be a no-op (lines without injected text and without wrapping). Doing now the same optimization directly in code.
• removes check for converting model range to view range that was added to fix Decorations that are ending on word-wrapped line leak to the next line whitespace #50163 since this is no longer necessary thanks to the affinity.
• in normalizePosition, I found it easier to handle the Left and Right affinities directly in the method, without converting to a model position and back. Maybe I'm missing something, but that seemed to be correct.

Let me know how you feel about my changes and if they seem correct to you, then please merge.

There is one follow-up item, but we can file it under an issue separately:

• If we want to move over inline completions to use injected text, we must figure out how to set ITextContentData.mightBeForeignElement to true for injected text or, even better, add a new property e.g. ITextContentData.injectedTextDecorationId?: string | null. This should then be adopted in inlineCompletionsHoverParticipant.ts. The hover participant wants to know if the mouse is over injected text. This information needs to be captured in _doHitTest, around doing normalization.

[hediet]

in normalizePosition, I found it easier to handle the Left and Right affinities directly in the method, without converting to a model position and back. Maybe I'm missing something, but that seemed to be correct.

Unfortuntately, I think this is not correct. Multiple injected texts might be injected at the same model position. Then you need to go at the start/end of the first/last injected text at that model position. When no position affinity is set (=none), there is nothing against setting a cursor between injected texts that are injected to the same position.

Even a wrapping point could be immediately before/after an injected text. Left/right normalization must skip it, otherwise move left/right is not going to work properly.
Thats why a view -> model -> view roundtrip imo is the best solution here.

[alexdima]

Multiple injected texts might be injected at the same model position

👍 Aha! Very good point! I think this._lineBreakData.getInjectedTextAt can be renamed/changed to do the normalization itself. It doesn't seem to have any other callers. It could be renamed to normalizeOffset() and it could deal inside of it with the touching injected text. What do you think about that?

Even a wrapping point could be immediately before/after an injected text. Left/right normalization must skip it, otherwise move left/right is not going to work properly.

Could that be resolved by invoking this._lineBreakData.getOutputPositionOfOffsetInUnwrappedLine with the correct affinity? I moved that method out of a case where the affinity was always PositionAffinity.None so the parameter was not passed in, but if we'd pass in the affinity, I think that would take care of the wrapping points.

Thats why a view -> model -> view roundtrip imo is the best solution here.

I think this would be correct, but I have this intuition that this might be error-prone (or lossy?). If we ever want to add text elision, doing the roundtrip might bring up more problems.

[hediet]

What do you think about that?

You mean for both left/right/no-affinity normalization or only for no-affinity?
If the latter, then I agree. However, internally that method might still make sense as finding the injected text at a position is easier to understand than normalization.

[alexdima]

This is what I meant -- e03e480

[Kingwl]

Cool! Thanks for the work!