Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support navigating notebook chat change from notebook diff editor #241760

Merged
merged 1 commit into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import { IEditorPane, SaveReason } from '../../../../common/editor.js';
import { IFilesConfigurationService } from '../../../../services/filesConfiguration/common/filesConfigurationService.js';
import { SnapshotContext } from '../../../../services/workingCopy/common/fileWorkingCopy.js';
import { ChatEditingNotebookFileSystemProvider } from '../../../notebook/browser/contrib/chatEdit/chatEditingNotebookFileSystemProvider.js';
import { NotebookTextDiffEditor } from '../../../notebook/browser/diff/notebookDiffEditor.js';
import { INotebookTextDiffEditor } from '../../../notebook/browser/diff/notebookDiffEditorBrowser.js';
import { getNotebookEditorFromEditorPane } from '../../../notebook/browser/notebookBrowser.js';
import { NotebookCellTextModel } from '../../../notebook/common/model/notebookCellTextModel.js';
import { NotebookTextModel } from '../../../notebook/common/model/notebookTextModel.js';
Expand All @@ -52,7 +54,7 @@ import { IChatResponseModel } from '../../common/chatModel.js';
import { IChatService } from '../../common/chatService.js';
import { IDocumentDiff2 } from './chatEditingCodeEditorIntegration.js';
import { AbstractChatEditingModifiedFileEntry, IModifiedEntryTelemetryInfo, ISnapshotEntry, pendingRewriteMinimap } from './chatEditingModifiedFileEntry.js';
import { ChatEditingNotebookEditorIntegration, countChanges, ICellDiffInfo, sortCellChanges } from './chatEditingNotebookEditorIntegration.js';
import { ChatEditingNotebookDiffEditorIntegration, ChatEditingNotebookEditorIntegration, countChanges, ICellDiffInfo, sortCellChanges } from './chatEditingNotebookEditorIntegration.js';
import { ChatEditingSnapshotTextModelContentProvider } from './chatEditingTextModelContentProviders.js';


Expand Down Expand Up @@ -255,6 +257,10 @@ export class ChatEditingModifiedNotebookEntry extends AbstractChatEditingModifie

protected override _createEditorIntegration(editor: IEditorPane): IModifiedFileEntryEditorIntegration {
const notebookEditor = getNotebookEditorFromEditorPane(editor);
if (!notebookEditor && editor.getId() === NotebookTextDiffEditor.ID) {
const diffEditor = (editor.getControl() as INotebookTextDiffEditor);
return this._instantiationService.createInstance(ChatEditingNotebookDiffEditorIntegration, diffEditor, this._cellDiffInfo);
}
assertType(notebookEditor);
return this._instantiationService.createInstance(ChatEditingNotebookEditorIntegration, this, notebookEditor, this.modifiedModel, this.originalModel, this._cellDiffInfo);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { IResourceDiffEditorInput } from '../../../../common/editor.js';
import { IEditorService } from '../../../../services/editor/common/editorService.js';
import { NotebookDeletedCellDecorator } from '../../../notebook/browser/diff/inlineDiff/notebookDeletedCellDecorator.js';
import { NotebookInsertedCellDecorator } from '../../../notebook/browser/diff/inlineDiff/notebookInsertedCellDecorator.js';
import { INotebookTextDiffEditor } from '../../../notebook/browser/diff/notebookDiffEditorBrowser.js';
import { INotebookEditor } from '../../../notebook/browser/notebookBrowser.js';
import { NotebookCellTextModel } from '../../../notebook/common/model/notebookCellTextModel.js';
import { NotebookTextModel } from '../../../notebook/common/model/notebookTextModel.js';
Expand Down Expand Up @@ -454,6 +455,76 @@ export class ChatEditingNotebookEditorIntegration extends Disposable implements

}
}
export class ChatEditingNotebookDiffEditorIntegration extends Disposable implements IModifiedFileEntryEditorIntegration {
private readonly _currentIndex = observableValue(this, -1);
readonly currentIndex: IObservable<number> = this._currentIndex;

constructor(
private readonly notebookDiffEditor: INotebookTextDiffEditor,
private readonly cellChanges: IObservable<ICellDiffInfo[]>
) {
super();

this._store.add(autorun(r => {
const index = notebookDiffEditor.currentChangedIndex.read(r);
const numberOfCellChanges = cellChanges.read(r).filter(c => !c.diff.identical);
if (numberOfCellChanges.length && index >= 0 && index < numberOfCellChanges.length) {
// Notebook Diff editor only supports navigating through changes to cells.
// However in chat we take changes to lines in the cells into account.
// So if we're on the second cell and first cell has 3 changes, then we're on the 4th change.
const changesSoFar = countChanges(numberOfCellChanges.slice(0, index + 1));
this._currentIndex.set(changesSoFar - 1, undefined);
} else {
this._currentIndex.set(-1, undefined);
}
}));
}

reveal(firstOrLast: boolean): void {
const changes = sortCellChanges(this.cellChanges.get().filter(c => c.type !== 'unchanged'));
if (!changes.length) {
return undefined;
}
if (firstOrLast) {
this.notebookDiffEditor.firstChange();
} else {
this.notebookDiffEditor.lastChange();
}
}

next(_wrap: boolean): boolean {
const changes = this.cellChanges.get().filter(c => !c.diff.identical).length;
if (this.notebookDiffEditor.currentChangedIndex.get() === changes - 1) {
return false;
}
this.notebookDiffEditor.nextChange();
return true;
}

previous(_wrap: boolean): boolean {
const changes = this.cellChanges.get().filter(c => !c.diff.identical).length;
if (this.notebookDiffEditor.currentChangedIndex.get() === changes - 1) {
return false;
}
this.notebookDiffEditor.nextChange();
return true;
}

enableAccessibleDiffView(): void {
//
}
acceptNearestChange(change: IModifiedFileEntryChangeHunk): void {
change.accept();
this.next(true);
}
rejectNearestChange(change: IModifiedFileEntryChangeHunk): void {
change.reject();
this.next(true);
}
async toggleDiff(_change: IModifiedFileEntryChangeHunk | undefined): Promise<void> {
//
}
}

export function countChanges(changes: ICellDiffInfo[]): number {
return changes.reduce((count, diff) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import { INotebookService } from '../../common/notebookService.js';
import { DiffEditorHeightCalculatorService, IDiffEditorHeightCalculatorService } from './editorHeightCalculator.js';
import { IEditorService } from '../../../../services/editor/common/editorService.js';
import { NotebookInlineDiffWidget } from './inlineDiff/notebookInlineDiffWidget.js';
import { IObservable, observableValue } from '../../../../../base/common/observable.js';

const $ = DOM.$;

Expand Down Expand Up @@ -153,6 +154,8 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
get isDisposed() {
return this._isDisposed;
}
private readonly _currentChangedIndex = observableValue(this, -1);
readonly currentChangedIndex: IObservable<number> = this._currentChangedIndex;

constructor(
group: IEditorGroup,
Expand Down Expand Up @@ -561,6 +564,17 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
updateInsets();
}));

this._localStore.add(this._list.onDidChangeFocus((e) => {
if (e.indexes.length && this.notebookDiffViewModel && e.indexes[0] < this.notebookDiffViewModel.items.length) {
const selectedItem = this.notebookDiffViewModel.items[e.indexes[0]];
const changedItems = this.notebookDiffViewModel.items.filter(item => item.type !== 'unchanged' && item.type !== 'unchangedMetadata' && item.type !== 'placeholder');
if (selectedItem && selectedItem?.type !== 'placeholder' && selectedItem?.type !== 'unchanged' && selectedItem?.type !== 'unchangedMetadata') {
return this._currentChangedIndex.set(changedItems.indexOf(selectedItem), undefined);
}
}
return this._currentChangedIndex.set(-1, undefined);
}));

this._localStore.add(this._eventDispatcher.onDidChangeCellLayout(() => {
updateInsets();
}));
Expand Down Expand Up @@ -700,6 +714,33 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
this._list.triggerScrollFromMouseWheelEvent(event);
}

firstChange(): void {
if (!this.notebookDiffViewModel) {
return;
}
// go to the first one
const currentViewModels = this.notebookDiffViewModel.items;
const index = currentViewModels.findIndex(vm => vm.type !== 'unchanged' && vm.type !== 'unchangedMetadata' && vm.type !== 'placeholder');
if (index >= 0) {
this._list.setFocus([index]);
this._list.reveal(index);
}
}

lastChange(): void {
if (!this.notebookDiffViewModel) {
return;
}
// go to the first one
const currentViewModels = this.notebookDiffViewModel.items;
const item = currentViewModels.slice().reverse().find(vm => vm.type !== 'unchanged' && vm.type !== 'unchangedMetadata' && vm.type !== 'placeholder');
const index = item ? currentViewModels.indexOf(item) : -1;
if (index >= 0) {
this._list.setFocus([index]);
this._list.reveal(index);
}
}

previousChange(): void {
if (!this.notebookDiffViewModel) {
return;
Expand All @@ -715,7 +756,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
const currentViewModels = this.notebookDiffViewModel.items;
while (prevChangeIndex >= 0) {
const vm = currentViewModels[prevChangeIndex];
if (vm.type !== 'unchanged' && vm.type !== 'placeholder') {
if (vm.type !== 'unchanged' && vm.type !== 'unchangedMetadata' && vm.type !== 'placeholder') {
break;
}

Expand All @@ -727,7 +768,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
this._list.reveal(prevChangeIndex);
} else {
// go to the last one
const index = findLastIdx(currentViewModels, vm => vm.type !== 'unchanged' && vm.type !== 'placeholder');
const index = findLastIdx(currentViewModels, vm => vm.type !== 'unchanged' && vm.type !== 'unchangedMetadata' && vm.type !== 'placeholder');
if (index >= 0) {
this._list.setFocus([index]);
this._list.reveal(index);
Expand All @@ -750,7 +791,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
const currentViewModels = this.notebookDiffViewModel.items;
while (nextChangeIndex < currentViewModels.length) {
const vm = currentViewModels[nextChangeIndex];
if (vm.type !== 'unchanged' && vm.type !== 'placeholder') {
if (vm.type !== 'unchanged' && vm.type !== 'unchangedMetadata' && vm.type !== 'placeholder') {
break;
}

Expand All @@ -762,7 +803,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD
this._list.reveal(nextChangeIndex);
} else {
// go to the first one
const index = currentViewModels.findIndex(vm => vm.type !== 'unchanged' && vm.type !== 'placeholder');
const index = currentViewModels.findIndex(vm => vm.type !== 'unchanged' && vm.type !== 'unchangedMetadata' && vm.type !== 'placeholder');
if (index >= 0) {
this._list.setFocus([index]);
this._list.reveal(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { WorkbenchToolBar } from '../../../../../platform/actions/browser/toolba
import { DiffEditorWidget } from '../../../../../editor/browser/widget/diffEditor/diffEditorWidget.js';
import { CancellationToken } from '../../../../../base/common/cancellation.js';
import { localize } from '../../../../../nls.js';
import { IObservable } from '../../../../../base/common/observable.js';

export enum DiffSide {
Original = 0,
Expand All @@ -32,6 +33,7 @@ export interface INotebookTextDiffEditor {
notebookOptions: NotebookOptions;
readonly textModel?: NotebookTextModel;
inlineNotebookEditor: INotebookEditor | undefined;
readonly currentChangedIndex: IObservable<number>;
onMouseUp: Event<{ readonly event: MouseEvent; readonly target: IDiffElementViewModelBase }>;
onDidScroll: Event<void>;
onDidDynamicOutputRendered: Event<{ cell: IGenericCellViewModel; output: ICellOutputViewModel }>;
Expand All @@ -54,6 +56,8 @@ export interface INotebookTextDiffEditor {
focusNextNotebookCell(cell: IGenericCellViewModel, focus: 'editor' | 'container' | 'output'): Promise<void>;
updateOutputHeight(cellInfo: ICommonCellInfo, output: ICellOutputViewModel, height: number, isInit: boolean): void;
deltaCellOutputContainerClassNames(diffSide: DiffSide, cellId: string, added: string[], removed: string[]): void;
firstChange(): void;
lastChange(): void;
previousChange(): void;
nextChange(): void;
toggleInlineView(): void;
Expand Down
Loading