Autocomplete: trigger multiline completions on empty block only #1052
Conversation
There was a problem hiding this comment.
Hmmm interesting... I can still reproduce the issue but only on line n -> n+1 , it doesn't seem to be an issue for the lines after that (see loom for details) ...?🤔
(It's kinda late here so i haven't really paid attention to the change, will look at it more closel.y tmr and let you know if i find anything!)
Loom: https://www.loom.com/share/9790bc66ac9847af920d92a1720e62a9?sid=a14685fa-79c3-4ba1-a48f-21b11cc9a309
Current:
Infill prompt
|
Hey @abeatrix, thanks for sharing the Loom video! I'm confused because you demonstrate single-line completions inside a non-empty block unrelated to changes in this PR. However, I see the issue with duplicated suffixes in your video. Could you elaborate on what problem you refer to above? |
| // Only trigger multiline suggestions when the next non-empty line is indented less | ||
| // than the block start line (the newly created block is empty). | ||
| indentation(currentLinePrefix) >= indentation(nextNonEmptyLine) |
There was a problem hiding this comment.
Should we extract this into a variable and use the same var in the check below to avoid duplication?
There was a problem hiding this comment.
I wanted to do that, but we rely on the currentLinePrefix indentation for the openingBracketMatch and prevNonEmptyLine indentation for the empty line case.
| @@ -82,33 +79,55 @@ describe('[getInlineCompletions] triggers', () => { | |||
| }, | |||
| }) | |||
| ) | |||
| expect(requests).toHaveLength(1) | |||
| expect(requests[0].stopSequences).toContain(MULTILINE_STOP_SEQUENCE) | |||
| expect(requests).toBeSingleLine() | |||
There was a problem hiding this comment.
Hey @abeatrix, thanks for sharing the Loom video!
I'm confused because you demonstrate single-line completions inside a non-empty block unrelated to changes in this PR. However, I see the issue with duplicated suffixes in your video. Could you elaborate on what problem you refer to above?
Sorry I just rebuilt and tried again this morning, and I can't reproduce the issue I posted to you anymore so maybe it was a caching issue? I can open a new issue if it comes back to me but like you mentioned, it's not what your PR is trying to cover so LGTM! Thanks for clarifying!
Context
enableExtendedTriggers(non-empty-line multi-line triggers): do not trigger multi-line completions for opening brackets if the content already exists in the block.vitestcustom matchers for multi vs sigle-line completions checks.Test plan
Added unit-tests.