sublimehq / sublime_text

Issue tracker for Sublime Text
https://www.sublimetext.com
814 stars 40 forks source link

Add ability to content stream into Phantom #6493

Closed yaroslavyaroslav closed 1 month ago

yaroslavyaroslav commented 2 months ago

Problem description

Phantom instance is incapable of being updated. When working with streaming data source (literally any AI backend) content presents by a streaming, and for sake of a better UX it should be presented that way.

Phantom is quite well fit to be used as one of such presentations, if only there were no limitations related to updating content of already presented Phantoms.

Preferred solution

None of Phantom neither PhantomSet provides such API call to update given Phantom by its id. Preferred solution would be to provide any of those:

First approach example:

content: str = "aksjdhfalksdjhfalksjhdf"
phantom = sublime.Phantom(line_beginning, "", sublime.PhantomLayout.INLINE, None)
ph_set = sublime.PhantomSet.update([phantom])

for char in content:
    phantom.content += char
    ph_set.update([phantom])

Second approach example:

content: str = "aksjdhfalksdjhfalksjhdf"
phantom = sublime.Phantom(line_beginning, "", sublime.PhantomLayout.INLINE, None)
ph_set = sublime.PhantomSet.update([phantom])

tmp_content = ''

for char in content:
    tmp_content += char
    phantom.update_with_content(tmp_content)

I consider the second approach as more straightforward.

Alternatives

Currently required behaviour could be achieved by reinstantiating a phantom in PhantomSet on every step in the loop, which leads to quite jittery UI behaviour.

content: str = "aksjdhfalksdjhfalksjhdf"
phantom = sublime.Phantom(line_beginning, "", sublime.PhantomLayout.INLINE, None)
ph_set = sublime.PhantomSet.update([phantom])

for char in content:
    phantom.content += char
    ph_set.update([])
    ph_set.update([phantom])

Additional Information

No response

BenjaminSchaaf commented 2 months ago

This is unfortunately quite a complex change - internally phantoms fundamentally cannot be updated. Mini-auto-complete erases the phantom and re-creates it every character.

Similarly the content of the phantom cannot be streamed as our HTML engine doesn't support streaming in any way, so it requires parsing the entire phantom every time.

yaroslavyaroslav commented 1 month ago

I decided to reopen this issue for further discussion. So I got the point that the only way to make phantoms handle content streaming is the one that I'm actually using. Which is:

content = PHANTOM_TEMPLATE.format(streaming_content=self.completion)

phantom = (
    self.phantom
    if self.phantom
    else Phantom(line_beginning, content, PhantomLayout.BLOCK, self.close_phantom)
)

self.phantom_set.update([phantom])

And that's fine with me. But while I'm doing so on each phantom_set.update() call phantom get's erased and reapplied which ends up with jittery UI behaviour both buffer content and buffer scroll bar.

So the current request is, @BenjaminSchaaf could this particular case of phantom updates be smoothed from user perspective? I actually not sure how can this be handled in particular, but from the logic of phantom updating, I'm seeing this as skipping to draw the remove (no phantom shown) part and jumping right to reapply (updated phantom shown part) part.

rchl commented 1 month ago

Not sure how your whole code looks like exactly but doing the update from the main thread should provide pretty much flicker-less experience.

yaroslavyaroslav commented 1 month ago

Yeah, calling update method through sublime.set_timeout fixed the issue, thank you.