From d2d8b929bedb1f34820a64e73dd3a8cf20417b56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krassowski?= <5832902+krassowski@users.noreply.github.com> Date: Wed, 3 Jul 2024 15:43:15 +0100 Subject: [PATCH] Backport PR #16507: Add customisation options to prevent inline completer resizing aggressively --- .../schema/inline-completer.json | 27 ++++++ packages/completer/src/ghost.ts | 87 ++++++++++++++++--- packages/completer/src/inline.ts | 34 +++++++- packages/completer/src/tokens.ts | 16 ++++ packages/completer/style/base.css | 19 ++++ packages/completer/test/inline.spec.ts | 69 +++++++++++++++ 6 files changed, 238 insertions(+), 14 deletions(-) diff --git a/packages/completer-extension/schema/inline-completer.json b/packages/completer-extension/schema/inline-completer.json index de4dc2ef9d96..9eed7309b223 100644 --- a/packages/completer-extension/schema/inline-completer.json +++ b/packages/completer-extension/schema/inline-completer.json @@ -61,6 +61,33 @@ { "const": "uncover", "title": "Uncover" } ], "default": "uncover" + }, + "minLines": { + "title": "Reserve lines for inline completion", + "description": "Number of lines to reserve for the ghost text with inline completion suggestion.", + "type": "number", + "default": 0, + "minimum": 0 + }, + "maxLines": { + "title": "Limit inline completion lines", + "description": "Number of lines of inline completion to show before collapsing. Setting zero disables the limit.", + "type": "number", + "default": 0, + "minimum": 0 + }, + "reserveSpaceForLongest": { + "title": "Reserve space for the longest candidate", + "description": "When multiple completions are returned, reserve blank space for up to as many lines as in the longest completion candidate to avoid resizing editor when cycling between the suggestions.", + "type": "boolean", + "default": false + }, + "editorResizeDelay": { + "title": "Editor resize delay", + "description": "When an inline completion gets cancelled the editor may change its size rapidly. When typing in the editor, the completions may get dismissed frequently causing a noticeable jitter of the editor height. Adding a delay prevents the jitter on typing. The value should be in milliseconds.", + "type": "number", + "default": 1000, + "minimum": 0 } }, "additionalProperties": false, diff --git a/packages/completer/src/ghost.ts b/packages/completer/src/ghost.ts index 168bc131774c..7caf59237613 100644 --- a/packages/completer/src/ghost.ts +++ b/packages/completer/src/ghost.ts @@ -14,6 +14,7 @@ const TRANSIENT_LETTER_SPACER_CLASS = 'jp-GhostText-letterSpacer'; const GHOST_TEXT_CLASS = 'jp-GhostText'; const STREAMED_TOKEN_CLASS = 'jp-GhostText-streamedToken'; const STREAMING_INDICATOR_CLASS = 'jp-GhostText-streamingIndicator'; +const HIDDEN_LINES_CLASS = 'jp-GhostText-hiddenLines'; /** * Ghost text content and placement. @@ -39,6 +40,14 @@ export interface IGhostText { * Whether streaming is in progress. */ streaming?: boolean; + /** + * Maximum number of lines to show. + */ + maxLines?: number; + /** + * Minimum number of lines to reserve (to avoid frequent resizing). + */ + minLines?: number; /** * Callback to execute when pointer enters the boundary of the ghost text. */ @@ -59,6 +68,16 @@ export class GhostTextManager { */ static streamingAnimation: 'none' | 'uncover' = 'uncover'; + /** + * Delay for removal of line spacer. + */ + static spacerRemovalDelay: number = 700; + + /** + * Duration for line spacer removal. + */ + static spacerRemovalDuration: number = 300; + /** * Place ghost text in an editor. */ @@ -140,33 +159,72 @@ class GhostTextWidget extends WidgetType { } private _updateDOM(dom: HTMLElement) { - const content = this.content; - - if (this.isSpacer) { - dom.innerText = content; - return; - } - + let content = this.content; + let hiddenContent = ''; let addition = this.options.addedPart; + if (addition) { if (addition.startsWith('\n')) { // Show the new line straight away to ensure proper positioning. addition = addition.substring(1); } - dom.innerText = content.substring(0, content.length - addition.length); + content = content.substring(0, content.length - addition.length); + } + + if (this.options.maxLines) { + // Split into content to show immediately and the hidden part + const lines = content.split('\n'); + content = lines.slice(0, this.options.maxLines).join('\n'); + hiddenContent = lines.slice(this.options.maxLines).join('\n'); + } + + const minLines = Math.min( + this.options.minLines ?? 0, + this.options.maxLines ?? Infinity + ); + const linesToAdd = Math.max(0, minLines - content.split('\n').length + 1); + const placeHolderLines = new Array(linesToAdd).fill('').join('\n'); + + if (this.isSpacer) { + dom.innerText = content + placeHolderLines; + return; + } + dom.innerText = content; + + let streamedTokenHost = dom; + + if (hiddenContent.length > 0) { + const hiddenWrapper = document.createElement('span'); + hiddenWrapper.className = 'jp-GhostText-hiddenWrapper'; + dom.appendChild(hiddenWrapper); + const expandOnHover = document.createElement('span'); + expandOnHover.className = 'jp-GhostText-expandHidden'; + expandOnHover.innerText = '⇓'; + const hiddenPart = document.createElement('span'); + hiddenWrapper.appendChild(expandOnHover); + hiddenPart.className = HIDDEN_LINES_CLASS; + hiddenPart.innerText = '\n' + hiddenContent; + hiddenWrapper.appendChild(hiddenPart); + streamedTokenHost = hiddenPart; + } + + if (addition) { const addedPart = document.createElement('span'); addedPart.className = STREAMED_TOKEN_CLASS; addedPart.innerText = addition; - dom.appendChild(addedPart); - } else { - // just set text - dom.innerText = content; + streamedTokenHost.appendChild(addedPart); } + // Add "streaming-in-progress" indicator if (this.options.streaming) { const streamingIndicator = document.createElement('span'); streamingIndicator.className = STREAMING_INDICATOR_CLASS; - dom.appendChild(streamingIndicator); + streamedTokenHost.appendChild(streamingIndicator); + } + + if (placeHolderLines.length > 0) { + const placeholderLinesNode = document.createTextNode(placeHolderLines); + streamedTokenHost.appendChild(placeholderLinesNode); } } destroy(dom: HTMLElement) { @@ -206,6 +264,9 @@ class TransientLineSpacerWidget extends TransientSpacerWidget { toDOM() { const wrap = super.toDOM(); wrap.classList.add(TRANSIENT_LINE_SPACER_CLASS); + wrap.style.animationDelay = GhostTextManager.spacerRemovalDelay + 'ms'; + wrap.style.animationDuration = + GhostTextManager.spacerRemovalDuration + 'ms'; return wrap; } } diff --git a/packages/completer/src/inline.ts b/packages/completer/src/inline.ts index a69838c98386..03630124f2d0 100644 --- a/packages/completer/src/inline.ts +++ b/packages/completer/src/inline.ts @@ -199,6 +199,17 @@ export class InlineCompleter extends Widget { this._updateShortcutsVisibility(); } GhostTextManager.streamingAnimation = settings.streamingAnimation; + GhostTextManager.spacerRemovalDelay = Math.max( + 0, + settings.editorResizeDelay - 300 + ); + GhostTextManager.spacerRemovalDuration = Math.max( + 0, + Math.min(300, settings.editorResizeDelay - 300) + ); + this._minLines = settings.minLines; + this._maxLines = settings.maxLines; + this._reserveSpaceForLongest = settings.reserveSpaceForLongest; } /** @@ -408,12 +419,26 @@ export class InlineCompleter extends Widget { } const view = (editor as CodeMirrorEditor).editor; + + let minLines: number; + if (this._reserveSpaceForLongest) { + const items = this.model?.completions?.items ?? []; + const longest = Math.max( + ...items.map(i => i.insertText.split('\n').length) + ); + minLines = Math.max(this._minLines, longest); + } else { + minLines = this._minLines; + } + this._ghostManager.placeGhost(view, { from: editor.getOffsetAt(model.cursor), content: text, providerId: item.provider.identifier, addedPart: item.lastStreamed, streaming: item.streaming, + minLines: minLines, + maxLines: this._maxLines, onPointerOver: this._onPointerOverGhost.bind(this), onPointerLeave: this._onPointerLeaveGhost.bind(this) }); @@ -487,6 +512,8 @@ export class InlineCompleter extends Widget { private _editor: CodeEditor.IEditor | null | undefined = null; private _ghostManager: GhostTextManager; private _lastItem: CompletionHandler.IInlineItem | null = null; + private _maxLines: number; + private _minLines: number; private _model: InlineCompleter.IModel | null = null; private _providerWidget = new Widget(); private _showShortcuts = InlineCompleter.defaultSettings.showShortcuts; @@ -495,6 +522,7 @@ export class InlineCompleter extends Widget { private _trans: TranslationBundle; private _toolbar = new Toolbar(); private _progressBar: HTMLElement; + private _reserveSpaceForLongest: boolean; } /** @@ -534,7 +562,11 @@ export namespace InlineCompleter { showWidget: 'onHover', showShortcuts: true, streamingAnimation: 'uncover', - providers: {} + providers: {}, + minLines: 2, + maxLines: 4, + editorResizeDelay: 1000, + reserveSpaceForLongest: false }; /** diff --git a/packages/completer/src/tokens.ts b/packages/completer/src/tokens.ts index 5254202dd5b0..2e53cb68a172 100644 --- a/packages/completer/src/tokens.ts +++ b/packages/completer/src/tokens.ts @@ -429,6 +429,22 @@ export interface IInlineCompleterSettings { * Transition effect used when streaming tokens from model. */ streamingAnimation: 'none' | 'uncover'; + /** + * Minimum lines to show. + */ + minLines: number; + /** + * Maximum lines to show. + */ + maxLines: number; + /** + * Delay between resizing the editor after an incline completion was cancelled. + */ + editorResizeDelay: number; + /* + * Reserve space for the longest of the completions candidates. + */ + reserveSpaceForLongest: boolean; /** * Provider settings. */ diff --git a/packages/completer/style/base.css b/packages/completer/style/base.css index 258c9b2d1bbb..d9cde8d4884b 100644 --- a/packages/completer/style/base.css +++ b/packages/completer/style/base.css @@ -227,6 +227,7 @@ } .jp-GhostText-lineSpacer { + /* duration and delay are overwritten by inline styles */ animation: jp-GhostText-hide 300ms 700ms ease-out forwards; } @@ -240,6 +241,24 @@ } } +.jp-GhostText-expandHidden { + border: 1px solid var(--jp-border-color0); + border-radius: var(--jp-border-radius); + background: var(--jp-layout-color0); + color: var(--jp-content-font-color3); + padding: 0 4px; + margin: 0 4px; + cursor: default; +} + +.jp-GhostText-hiddenWrapper:hover > .jp-GhostText-hiddenLines { + display: inline; +} + +.jp-GhostText-hiddenLines { + display: none; +} + .jp-GhostText[data-animation='uncover'] { position: relative; } diff --git a/packages/completer/test/inline.spec.ts b/packages/completer/test/inline.spec.ts index 675e2ae3c194..bacffe7f56d3 100644 --- a/packages/completer/test/inline.spec.ts +++ b/packages/completer/test/inline.spec.ts @@ -14,6 +14,8 @@ import { Widget } from '@lumino/widgets'; import { MessageLoop } from '@lumino/messaging'; import { Doc, Text } from 'yjs'; +const GHOST_TEXT_CLASS = 'jp-GhostText'; + describe('completer/inline', () => { const exampleProvider: IInlineCompletionProvider = { name: 'An inline provider', @@ -36,6 +38,8 @@ describe('completer/inline', () => { let editorWidget: CodeEditorWrapper; let model: InlineCompleter.Model; let suggestionsAbc: CompletionHandler.IInlineItem[]; + const findInHost = (selector: string) => + editorWidget.editor.host.querySelector(selector); beforeEach(() => { editorWidget = createEditorWidget(); @@ -151,6 +155,71 @@ describe('completer/inline', () => { }); expect(completer.node.dataset.showShortcuts).toBe('false'); }); + + it('`maxLines` should limit the number of lines visible', async () => { + Widget.attach(editorWidget, document.body); + Widget.attach(completer, document.body); + completer.configure({ + ...InlineCompleter.defaultSettings, + maxLines: 3 + }); + const item: CompletionHandler.IInlineItem = { + ...itemDefaults, + insertText: 'line1\nline2\nline3\nline4\nline5' + }; + model.setCompletions({ items: [item] }); + + const ghost = findInHost(`.${GHOST_TEXT_CLASS}`) as HTMLElement; + expect(ghost.innerText).toBe('line1\nline2\nline3'); + }); + + const getGhostTextContent = () => { + const ghost = findInHost(`.${GHOST_TEXT_CLASS}`) as HTMLElement; + // jest-dom does not support textContent/innerText properly, we need to extract it manually + return ( + ghost.innerText + + [...ghost.childNodes].map(node => node.textContent).join('') + ); + }; + + it('`minLines` should add empty lines when needed', async () => { + Widget.attach(editorWidget, document.body); + Widget.attach(completer, document.body); + completer.configure({ + ...InlineCompleter.defaultSettings, + minLines: 3 + }); + const item: CompletionHandler.IInlineItem = { + ...itemDefaults, + insertText: 'line1' + }; + model.setCompletions({ items: [item] }); + + expect(getGhostTextContent()).toBe('line1\n\n'); + }); + + it('`reserveSpaceForLongest` should add empty lines when needed', async () => { + Widget.attach(editorWidget, document.body); + Widget.attach(completer, document.body); + completer.configure({ + ...InlineCompleter.defaultSettings, + reserveSpaceForLongest: true + }); + model.setCompletions({ + items: [ + { + ...itemDefaults, + insertText: 'line1' + }, + { + ...itemDefaults, + insertText: 'line1\nline2\nline3' + } + ] + }); + + expect(getGhostTextContent()).toBe('line1\n\n'); + }); }); describe('#cycle()', () => {