Skip to content

Commit

Permalink
Support navigating notebook chat change in notebook diff editor
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonJayamanne
DonJayamanne committed Feb 24, 2025
1 parent 434ebfb commit 39df0b1
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 5 deletions.
8 changes: 7 additions & 1 deletion src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedNotebookEntry.ts
View file
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
71 changes: 71 additions & 0 deletions src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingNotebookEditorIntegration.ts
View file
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
49 changes: 45 additions & 4 deletions src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts
View file
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
4 changes: 4 additions & 0 deletions src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts
View file
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

0 comments on commit 39df0b1

Please sign in to comment.