Merge branch 'main' into dev/dmitriv/finalize-button-location-api

This commit is contained in:
Dmitriy Vasyura
2026-01-05 00:38:17 -08:00
committed by GitHub
4 changed files with 310 additions and 23 deletions

View File

@@ -29,6 +29,8 @@ import { ILanguageModelIgnoredFilesService } from '../../common/ignoredFiles.js'
import { getPromptsTypeForLanguageId } from '../../common/promptSyntax/promptTypes.js';
import { IChatWidget, IChatWidgetService } from '../chat.js';
import { IChatContextService } from '../contextContrib/chatContextService.js';
import { ITextModel } from '../../../../../editor/common/model.js';
import { IRange } from '../../../../../editor/common/core/range.js';
export class ChatImplicitContextContribution extends Disposable implements IWorkbenchContribution {
static readonly ID = 'chat.implicitContext';
@@ -210,18 +212,22 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
const selection = codeEditor?.getSelection();
const visibleRanges = codeEditor?.getVisibleRanges() || [];
newValue = activeCell.uri;
if (isEqual(codeEditor?.getModel()?.uri, activeCell.uri)) {
const cellModel = codeEditor?.getModel();
if (cellModel && isEqual(cellModel.uri, activeCell.uri)) {
if (selection && !selection.isEmpty()) {
newValue = { uri: activeCell.uri, range: selection } satisfies Location;
isSelection = true;
} else if (visibleRanges.length > 0) {
// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
// Something like a Location with an array of Ranges?
let range = visibleRanges[0];
visibleRanges.slice(1).forEach(r => {
range = range.plusRange(r);
});
newValue = { uri: activeCell.uri, range } satisfies Location;
// If the entire cell is visible, just use the cell URI, no need to specify range.
if (!isEntireCellVisible(cellModel, visibleRanges)) {
// Merge visible ranges. Maybe the reference value could actually be an array of Locations?
// Something like a Location with an array of Ranges?
let range = visibleRanges[0];
visibleRanges.slice(1).forEach(r => {
range = range.plusRange(r);
});
newValue = { uri: activeCell.uri, range } satisfies Location;
}
}
}
} else {
@@ -267,6 +273,13 @@ export class ChatImplicitContextContribution extends Disposable implements IWork
}
}
function isEntireCellVisible(cellModel: ITextModel, visibleRanges: IRange[]): boolean {
if (visibleRanges.length === 1 && visibleRanges[0].startLineNumber === 1 && visibleRanges[0].startColumn === 1 && visibleRanges[0].endLineNumber === cellModel.getLineCount() && visibleRanges[0].endColumn === cellModel.getLineMaxColumn(visibleRanges[0].endLineNumber)) {
return true;
}
return false;
}
export class ChatImplicitContext extends Disposable implements IChatRequestImplicitVariableEntry {
get id() {
if (URI.isUri(this.value)) {

View File

@@ -14,7 +14,7 @@ import { Disposable, DisposableStore } from '../../../../../base/common/lifecycl
import { Schemas } from '../../../../../base/common/network.js';
import { basename, dirname } from '../../../../../base/common/resources.js';
import { URI } from '../../../../../base/common/uri.js';
import { Location } from '../../../../../editor/common/languages.js';
import { isLocation, Location } from '../../../../../editor/common/languages.js';
import { ILanguageService } from '../../../../../editor/common/languages/language.js';
import { IModelService } from '../../../../../editor/common/services/model.js';
import { localize } from '../../../../../nls.js';
@@ -214,7 +214,11 @@ export class ImplicitContextAttachmentWidget extends Disposable {
this.attachmentModel.addContext(context);
} else {
const file = URI.isUri(this.attachment.value) ? this.attachment.value : this.attachment.value.uri;
this.attachmentModel.addFile(file);
if (file.scheme === Schemas.vscodeNotebookCell && isLocation(this.attachment.value)) {
this.attachmentModel.addFile(file, this.attachment.value.range);
} else {
this.attachmentModel.addFile(file);
}
}
this.widgetRef()?.focusInput();
}

View File

@@ -772,10 +772,13 @@ export class CodeCellLayout {
* observe transient Monaco content heights that reflect the current clipped layout (rather than
* the full input height). To keep the notebook list layout stable (avoiding overlapping cells
* while navigating/scrolling), we store the actual content height in `_establishedContentHeight`
* and reuse it for all layout reasons except `onDidContentSizeChange`. This prevents the editor
* from shrinking back to its initial height after content has been added (e.g., pasting text).
* When `onDidContentSizeChange` fires, we update `_establishedContentHeight` to reflect the new
* content size, which subsequent scroll events will then reuse.
* and reuse it for scroll-driven relayouts. This prevents the editor from shrinking back to its
* initial height after content has been added (e.g., pasting text) or when Monaco reports a
* transient smaller content height while the cell is clipped.
*
* We refresh `_establishedContentHeight` when the editor's content size changes
* (`onDidContentSizeChange`) and also when width/layout changes can affect wrapping-driven height
* (`viewCellLayoutChange`/`nbLayoutChange`).
* - Pointer-drag gating: while the user is holding the mouse button down in the editor (drag
* selection or potential drag selection), we avoid programmatic `editor.setScrollTop(...)` updates
* to prevent selection/scroll feedback loops and "stuck selection" behavior.
@@ -870,17 +873,51 @@ export class CodeCellLayout {
const elementTop = this.notebookEditor.getAbsoluteTopOfElement(this.viewCell);
const elementBottom = this.notebookEditor.getAbsoluteBottomOfElement(this.viewCell);
const elementHeight = this.notebookEditor.getHeightOfElement(this.viewCell);
const gotContentHeight = editor.getContentHeight();
// If we've already calculated the editor content height once before and the contents haven't changed, use that.
const fallbackEditorContentHeight = gotContentHeight === -1 ? Math.max(editor.getLayoutInfo().height, this._initialEditorDimension.height) : gotContentHeight;
let editorContentHeight: number;
if (this._initialized && reason !== 'onDidContentSizeChange') {
// Reuse the previously established content height to avoid transient Monaco content height changes during scroll
editorContentHeight = this._establishedContentHeight ?? fallbackEditorContentHeight;
} else {
// Update the established content height when content actually changes or during initialization
editorContentHeight = fallbackEditorContentHeight;
const isInit = !this._initialized && reason === 'init';
if (isInit) {
// CONTENT HEIGHT SELECTION (INIT)
// -------------------------------
// Editors are pooled and may be re-attached to different cells as the user scrolls.
// At the moment a pooled editor is first attached to a new cell, Monaco can still
// report the previous cell's `getContentHeight()` (for example a tall multi-line
// cell) even though the new cell only contains a single line. If we trusted that
// stale value here, the very first layout of the new cell would render with an
// oversized editor and visually overlap the next cell.
//
// To avoid this, the initial layout ignores `getContentHeight()` entirely and uses
// the notebook's own notion of the editor height for this cell
// (`_initialEditorDimension.height`). This value is derived from the cell model
// (line count + padding) and is stable across editor reuse. Once the model has
// been resolved and Monaco reports a real content height, subsequent layout
// reasons (`onDidContentSizeChange`, `viewCellLayoutChange`, `nbLayoutChange`)
// will refresh `_establishedContentHeight` in the normal way.
editorContentHeight = this._initialEditorDimension.height;
this._establishedContentHeight = editorContentHeight;
} else {
// CONTENT HEIGHT SELECTION (NON-INIT)
// -----------------------------------
// For all non-init reasons, we rely on Monaco's `getContentHeight()` together with
// `_establishedContentHeight` to keep the notebook list layout stable while
// scrolling and resizing:
// - `onDidContentSizeChange` / `viewCellLayoutChange` / `nbLayoutChange` update
// `_establishedContentHeight` to the latest full content height.
// - `nbDidScroll` reuses `_establishedContentHeight` so that transient, smaller
// values reported while the editor itself is clipped do not shrink the row
// height (which would otherwise cause overlapping cells).
const gotContentHeight = editor.getContentHeight();
// If we've already calculated the editor content height once before and the contents haven't changed, use that.
const fallbackEditorContentHeight = gotContentHeight === -1 ? Math.max(editor.getLayoutInfo().height, this._initialEditorDimension.height) : gotContentHeight;
const shouldRefreshContentHeight = !this._initialized || reason === 'onDidContentSizeChange' || reason === 'viewCellLayoutChange' || reason === 'nbLayoutChange';
if (shouldRefreshContentHeight) {
// Update the established content height when content changes, during initialization,
// or when width/layout changes can affect wrapping-driven height.
editorContentHeight = fallbackEditorContentHeight;
this._establishedContentHeight = editorContentHeight;
} else {
// Reuse the previously established content height to avoid transient Monaco content height changes during scroll
editorContentHeight = this._establishedContentHeight ?? fallbackEditorContentHeight;
}
}
const editorBottom = elementTop + this.viewCell.layoutInfo.outputContainerOffset;
const scrollBottom = this.notebookEditor.scrollBottom;

View File

@@ -496,6 +496,100 @@ suite('CellPart', () => {
);
});
test('CodeCellLayout refreshes content height on viewCellLayoutChange', () => {
const LINE_HEIGHT = 21;
const CELL_TOP_MARGIN = 6;
const CELL_OUTLINE_WIDTH = 1;
const STATUSBAR_HEIGHT = 22;
const VIEWPORT_HEIGHT = 1000;
const ELEMENT_TOP = 100;
const ELEMENT_HEIGHT = 1200;
const INITIAL_CONTENT_HEIGHT = 37;
const OUTPUT_CONTAINER_OFFSET = 300;
const UPDATED_CONTENT_HEIGHT = 200;
let contentHeight = INITIAL_CONTENT_HEIGHT;
const stubEditor = {
layoutCalls: [] as { width: number; height: number }[],
_lastScrollTopSet: -1,
getLayoutInfo: () => ({ width: 600, height: INITIAL_CONTENT_HEIGHT }),
getContentHeight: () => contentHeight,
layout: (dim: { width: number; height: number }) => {
stubEditor.layoutCalls.push(dim);
},
setScrollTop: (v: number) => {
stubEditor._lastScrollTopSet = v;
},
hasModel: () => true,
};
const editorPart = { style: { top: '' } };
const template: Partial<CodeCellRenderTemplate> = {
editor: stubEditor as unknown as ICodeEditor,
editorPart: editorPart as unknown as HTMLElement,
};
const viewCell: Partial<CodeCellViewModel> = {
isInputCollapsed: false,
layoutInfo: {
statusBarHeight: STATUSBAR_HEIGHT,
topMargin: CELL_TOP_MARGIN,
outlineWidth: CELL_OUTLINE_WIDTH,
editorHeight: INITIAL_CONTENT_HEIGHT,
outputContainerOffset: OUTPUT_CONTAINER_OFFSET,
editorWidth: 600,
} as unknown as CodeCellLayoutInfo,
};
const notebookEditor = {
scrollTop: 0,
get scrollBottom() {
return VIEWPORT_HEIGHT;
},
setScrollTop: (v: number) => {
/* no-op */
},
getLayoutInfo: () => ({
fontInfo: { lineHeight: LINE_HEIGHT },
height: VIEWPORT_HEIGHT,
stickyHeight: 0,
}),
getAbsoluteTopOfElement: () => ELEMENT_TOP,
getAbsoluteBottomOfElement: () => ELEMENT_TOP + OUTPUT_CONTAINER_OFFSET,
getHeightOfElement: () => ELEMENT_HEIGHT,
notebookOptions: {
getLayoutConfiguration: () => ({ editorTopPadding: 6 }),
},
};
const layout = new CodeCellLayout(
true,
notebookEditor as unknown as IActiveNotebookEditorDelegate,
viewCell as CodeCellViewModel,
template as CodeCellRenderTemplate,
{ debug: () => { } },
{ width: 600, height: INITIAL_CONTENT_HEIGHT }
);
layout.layoutEditor('init');
assert.strictEqual(stubEditor.layoutCalls.at(-1)?.height, INITIAL_CONTENT_HEIGHT);
// Simulate wrapping-driven height increase after width/layout settles.
contentHeight = UPDATED_CONTENT_HEIGHT;
layout.layoutEditor('viewCellLayoutChange');
assert.strictEqual(
stubEditor.layoutCalls.at(-1)?.height,
UPDATED_CONTENT_HEIGHT,
'viewCellLayoutChange should refresh the content height'
);
// Ensure subsequent scrolls still reuse the established (larger) height.
contentHeight = 50;
layout.layoutEditor('nbDidScroll');
assert.strictEqual(
stubEditor.layoutCalls.at(-1)?.height,
UPDATED_CONTENT_HEIGHT,
'nbDidScroll should reuse the refreshed content height'
);
});
test('CodeCellLayout maintains content height after paste when scrolling', () => {
/**
* Regression test for https://github.com/microsoft/vscode/issues/284524
@@ -709,4 +803,143 @@ suite('CellPart', () => {
'Expected editor.setScrollTop to resume once pointer is released'
);
});
test('CodeCellLayout init ignores stale pooled editor content height', () => {
/**
* Regression guard for fast-scroll overlap when editors are pooled.
*
* A Monaco editor instance can be reused between cells. If we trusted the pooled
* editor's `getContentHeight()` during the first layout of a new cell, a short
* cell might inherit a previous tall cell's content height and render with an
* oversized editor, visually overlapping the next cell. The layout should instead
* seed its initial content height from the cell's own initial editor dimension.
*/
const LINE_HEIGHT = 21;
const CELL_TOP_MARGIN = 6;
const CELL_OUTLINE_WIDTH = 1;
const STATUSBAR_HEIGHT = 22;
const VIEWPORT_HEIGHT = 400;
const ELEMENT_TOP = 100;
const ELEMENT_HEIGHT = 500;
const OUTPUT_CONTAINER_OFFSET = 200;
let pooledContentHeight = 200; // tall previous cell
const pooledEditor = {
layoutCalls: [] as { width: number; height: number }[],
_lastScrollTopSet: -1,
getLayoutInfo: () => ({ width: 600, height: pooledContentHeight }),
getContentHeight: () => pooledContentHeight,
layout: (dim: { width: number; height: number }) => {
pooledEditor.layoutCalls.push(dim);
},
setScrollTop: (v: number) => {
pooledEditor._lastScrollTopSet = v;
},
hasModel: () => true,
};
const editorPart = { style: { top: '' } };
const template: Partial<CodeCellRenderTemplate> = {
editor: pooledEditor as unknown as ICodeEditor,
editorPart: editorPart as unknown as HTMLElement,
};
// First, layout a tall cell to establish a large content height on the pooled editor.
const tallViewCell: Partial<CodeCellViewModel> = {
isInputCollapsed: false,
layoutInfo: {
statusBarHeight: STATUSBAR_HEIGHT,
topMargin: CELL_TOP_MARGIN,
outlineWidth: CELL_OUTLINE_WIDTH,
editorHeight: 200,
outputContainerOffset: OUTPUT_CONTAINER_OFFSET,
editorWidth: 600,
} as unknown as CodeCellLayoutInfo,
};
const tallNotebookEditor = {
scrollTop: 0,
get scrollBottom() {
return VIEWPORT_HEIGHT;
},
setScrollTop: (_v: number) => {
/* no-op for this test */
},
getLayoutInfo: () => ({
fontInfo: { lineHeight: LINE_HEIGHT },
height: VIEWPORT_HEIGHT,
stickyHeight: 0,
}),
getAbsoluteTopOfElement: () => ELEMENT_TOP,
getAbsoluteBottomOfElement: () => ELEMENT_TOP + OUTPUT_CONTAINER_OFFSET,
getHeightOfElement: () => ELEMENT_HEIGHT,
notebookOptions: {
getLayoutConfiguration: () => ({ editorTopPadding: 6 }),
},
};
const tallLayout = new CodeCellLayout(
true,
tallNotebookEditor as unknown as IActiveNotebookEditorDelegate,
tallViewCell as CodeCellViewModel,
template as CodeCellRenderTemplate,
{ debug: () => { } },
{ width: 600, height: 200 }
);
tallLayout.layoutEditor('init');
assert.strictEqual(
pooledEditor.layoutCalls.at(-1)?.height,
200,
'Expected tall cell to lay out using its own height'
);
// Now reuse the same editor for a short cell while leaving the pooled content height large.
pooledContentHeight = 200; // simulate stale value from previous cell
const shortViewCell: Partial<CodeCellViewModel> = {
isInputCollapsed: false,
layoutInfo: {
statusBarHeight: STATUSBAR_HEIGHT,
topMargin: CELL_TOP_MARGIN,
outlineWidth: CELL_OUTLINE_WIDTH,
editorHeight: 37,
outputContainerOffset: OUTPUT_CONTAINER_OFFSET,
editorWidth: 600,
} as unknown as CodeCellLayoutInfo,
};
const shortNotebookEditor = {
scrollTop: 0,
get scrollBottom() {
return VIEWPORT_HEIGHT;
},
setScrollTop: (_v: number) => {
/* no-op for this test */
},
getLayoutInfo: () => ({
fontInfo: { lineHeight: LINE_HEIGHT },
height: VIEWPORT_HEIGHT,
stickyHeight: 0,
}),
getAbsoluteTopOfElement: () => ELEMENT_TOP,
getAbsoluteBottomOfElement: () => ELEMENT_TOP + OUTPUT_CONTAINER_OFFSET,
getHeightOfElement: () => ELEMENT_HEIGHT,
notebookOptions: {
getLayoutConfiguration: () => ({ editorTopPadding: 6 }),
},
};
const shortLayout = new CodeCellLayout(
true,
shortNotebookEditor as unknown as IActiveNotebookEditorDelegate,
shortViewCell as CodeCellViewModel,
template as CodeCellRenderTemplate,
{ debug: () => { } },
{ width: 600, height: 37 }
);
shortLayout.layoutEditor('init');
assert.strictEqual(
pooledEditor.layoutCalls.at(-1)?.height,
37,
'Init layout for a short cell should use the cell\'s initial height, not the pooled editor\'s stale content height'
);
});
});