Fix assertion error

[justinmilner1]

Fixes #1909

Could not replicate the issue, but based on the stack trace it looks like a blank text value was making its way through this pathway.

I've added a check immediately prior to the instantiation of the renderer to prevent this.

It's possible the issue was arising from the line immediately above:
val text = (if (startsInline) lines.drop(1) else lines).dropWhile { it.isBlank() }.joinToString("\n")

Test plan

I ran the extension and verified via logging that the block was being executed as expected with autocomplete.

[justinmilner1]

Hey again @pkukielka :D looks like you worked on this section as well. I'm looking for another review.

[pkukielka]

Hey again @pkukielka :D looks like you worked on this section as well. I'm looking for another review.

Hey @justinmilner1 , thanks for looking into this!

I think it is good place to fix it but small correction may be needed.
The way it works is that there are two types of inlays, single line (which is rendered as the continuation of existing text in a line) and multiline (which is rendered bellow).

There might be a situation, where multiline completion is indeed blank, but single line is not.
So I think it should more or less look like that:

if (text.isBlank()) {
    if (!startsInline) {
        logger.warn("autocomplete returned blank suggestions")
        return
    }
} else {
    val renderer = CodyAutocompleteBlockElementRenderer(text, items, editor)
    val inlay2 =
          inlayModel.addBlockElement(
              /* offset = */ cursorOffset,
              /* relatesToPrecedingText = */ true,
              /* showAbove = */ false,
              /* priority = */ Int.MAX_VALUE,
              /* renderer = */ renderer)
    if (inlay == null) {
        inlay = inlay2
    }
}

It starts to be a bit convoluted.
Looking at those renderers implementation I think we could have one renderer which handles both cases and that would be much cleaner solution. I might give it a try later.

[justinmilner1]

@pkukielka Just pushed a commit with a draft of the consolidated renderers and simplified logic in the manager.
This needs further review/testing, test changes, etc, but want to make sure this is what you envisioned first.

[justinmilner1]

@pkukielka Just did some more manual testing, and I don't see any affected automated tests, so I think it is ready for review

[justinmilner1]

@pkukielka Bumping this

[dominiccooney]

It may make sense to keep the Element/BlockElement/SingleLine classes, but clean them up (specifically, naming: SingleLine/Multiline, or BlockElement/InlineElement, instead of two different domains like SingleLine/BlockElement; and push the single line width calculations down to the single line class and leave it abstract in Element...)

If not, do we need CodyAutocompleteElementRenderer now, why not fold it all into CodyAutocompleteRenderer?

[dominiccooney]

Should we move this to the CodyAutocompleteRenderer?

Or at the very least, we could look at something like renderer.type instead of the startsInline we're hanging on to.

[pkukielka]

Hey, sorry for the long delay.

I'm not sure this is correct.
Multiline autocompletion can start inline, but then continue to the other lines.
So in fact 3 states are possible:

• single inline autocompletion ('addInlineElement')
• multiline completion which starts bellow current line (addBlockElement)
• multiline completion which starts inline but also continues to the other lines (addInlineElement + addBlockElement)