-
Notifications
You must be signed in to change notification settings - Fork 29.4k
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
Separate IModifiedFileEntry into text and notebook #234388
base: main
Are you sure you want to change the base?
Conversation
ef7fdc9
to
97cd534
Compare
97cd534
to
e41b3d3
Compare
if (entry.kind === 'text' && snapshotEntry.kind === 'text') { | ||
entry.restoreFromSnapshot(snapshotEntry); | ||
} else if (entry.kind === 'notebook' && snapshotEntry.kind === 'notebook') { | ||
throw new Error('Not implemented'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now have a way to capture/restore snapshots for notebooks
And given we have interfaces for each, we can have separate concrete implementations for each
} satisfies ISnapshotEntry; | ||
const deserializeSnapshotEntry = (entry: ISnapshotEntryDTO) => { | ||
if (entry.kind === 'notebook') { | ||
throw new Error('Not implemented'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do later, pending creation/restoring of snapshots for notebooks
@@ -491,7 +491,7 @@ class DiffHunkWidget implements IOverlayWidget { | |||
|
|||
|
|||
constructor( | |||
readonly entry: IModifiedFileEntry, | |||
readonly entry: IModifiedTextFileEntry, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class only deals with editors for text files, editors for notebooks are already handled separately.
restoreFromSnapshot(snapshot: ITextSnapshotEntry): void; | ||
} | ||
|
||
export interface IModifiedNotebookFileEntry extends IModifiedAnyFileEntry { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separation of IModifiedFileEntry into text and notebook.
Similarly the snapshot interfaces have been separated, giving each their own implementations
@@ -86,7 +86,7 @@ class NotebookChatEditorController extends Disposable { | |||
if (!model || !session) { | |||
return; | |||
} | |||
return session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri)); | |||
return session.entries.read(r).find(e => isEqual(e.modifiedURI, model.uri) && e.kind === 'text') as IModifiedTextFileEntry | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notebooks today only deal with the textual entries.
Once we bake in full support for notebooks, this will go away
import { IChatService } from '../../common/chatService.js'; | ||
import { ChatEditingSnapshotTextModelContentProvider, ChatEditingTextModelContentProvider } from './chatEditingTextModelContentProviders.js'; | ||
|
||
export class ChatEditingModifiedFileEntry extends Disposable implements IModifiedFileEntry { | ||
|
||
export class ChatEditingModifiedFileEntry extends Disposable implements IModifiedTextFileEntry { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan is to create a new ChatEditingModifiedNotebookFileEntry class with custom handling for notebooks
That will then be responsible for handling notebook edits and the like (however quite a lot will be shared between the two)
@@ -184,8 +184,6 @@ export class ChatEditorSaving extends Disposable implements IWorkbenchContributi | |||
} | |||
|
|||
private _reportSaved(entry: IModifiedFileEntry) { | |||
assertType(entry instanceof ChatEditingModifiedFileEntry); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This no longer applies, as we can have a separate implementation for notebooks.
No description provided.