Skip to content

Commit

Permalink
Merge pull request #181280 from microsoft/aamunger/scrapbookIW
Browse files Browse the repository at this point in the history
create a scratchpad untitled workingcopy
  • Loading branch information
amunger authored May 19, 2023
2 parents aa79bd7 + 4864d33 commit c97098e
Show file tree
Hide file tree
Showing 22 changed files with 588 additions and 103 deletions.
8 changes: 7 additions & 1 deletion src/vs/workbench/browser/parts/editor/editorGroupView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -982,9 +982,15 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
this._onWillOpenEditor.fire({ editor, groupId: this.id });

// Determine options
const pinned = options?.sticky
|| !this.accessor.partOptions.enablePreview
|| editor.isDirty()
|| (options?.pinned ?? typeof options?.index === 'number' /* unless specified, prefer to pin when opening with index */)
|| (typeof options?.index === 'number' && this.model.isSticky(options.index))
|| editor.hasCapability(EditorInputCapabilities.Scratchpad);
const openEditorOptions: IEditorOpenOptions = {
index: options ? options.index : undefined,
pinned: options?.sticky || !this.accessor.partOptions.enablePreview || editor.isDirty() || (options?.pinned ?? typeof options?.index === 'number' /* unless specified, prefer to pin when opening with index */) || (typeof options?.index === 'number' && this.model.isSticky(options.index)),
pinned,
sticky: options?.sticky || (typeof options?.index === 'number' && this.model.isSticky(options.index)),
active: this.count === 0 || !options || !options.inactive,
supportSideBySide: internalOptions?.supportSideBySide
Expand Down
10 changes: 5 additions & 5 deletions src/vs/workbench/browser/parts/editor/editorsObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { IEditorFactoryRegistry, IEditorIdentifier, GroupIdentifier, EditorExtensions, IEditorPartOptionsChangeEvent, EditorsOrder, GroupModelChangeKind } from 'vs/workbench/common/editor';
import { IEditorFactoryRegistry, IEditorIdentifier, GroupIdentifier, EditorExtensions, IEditorPartOptionsChangeEvent, EditorsOrder, GroupModelChangeKind, EditorInputCapabilities } from 'vs/workbench/common/editor';
import { EditorInput } from 'vs/workbench/common/editor/editorInput';
import { SideBySideEditorInput } from 'vs/workbench/common/editor/sideBySideEditorInput';
import { dispose, Disposable, DisposableStore } from 'vs/base/common/lifecycle';
Expand Down Expand Up @@ -345,8 +345,8 @@ export class EditorsObserver extends Disposable {
let mostRecentEditorsCountingForLimit: IEditorIdentifier[];
if (this.editorGroupsService.partOptions.limit?.excludeDirty) {
mostRecentEditorsCountingForLimit = mostRecentEditors.filter(({ editor }) => {
if (editor.isDirty() && !editor.isSaving()) {
return false;
if ((editor.isDirty() && !editor.isSaving()) || editor.hasCapability(EditorInputCapabilities.Scratchpad)) {
return false; // not dirty editors (unless in the process of saving) or scratchpads
}

return true;
Expand All @@ -361,8 +361,8 @@ export class EditorsObserver extends Disposable {

// Extract least recently used editors that can be closed
const leastRecentlyClosableEditors = mostRecentEditorsCountingForLimit.reverse().filter(({ editor, groupId }) => {
if (editor.isDirty() && !editor.isSaving()) {
return false; // not dirty editors (unless in the process of saving)
if ((editor.isDirty() && !editor.isSaving()) || editor.hasCapability(EditorInputCapabilities.Scratchpad)) {
return false; // not dirty editors (unless in the process of saving) or scratchpads
}

if (exclude && editor === exclude.editor && groupId === exclude.groupId) {
Expand Down
8 changes: 7 additions & 1 deletion src/vs/workbench/common/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,13 @@ export const enum EditorInputCapabilities {
* Signals that the editor is composed of multiple editors
* within.
*/
MultipleEditors = 1 << 8
MultipleEditors = 1 << 8,

/**
* Signals that the editor cannot be in a dirty state
* and may still have unsaved changes
*/
Scratchpad = 1 << 9
}

export type IUntypedEditorInput = IResourceEditorInput | ITextResourceEditorInput | IUntitledTextResourceEditorInput | IResourceDiffEditorInput | IResourceSideBySideEditorInput | IResourceMergeEditorInput;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ export class SearchEditorInput extends EditorInput {
readonly onDidChangeContent = input.onDidChangeContent;
readonly onDidSave = input.onDidSave;
isDirty(): boolean { return input.isDirty(); }
isModified(): boolean { return input.isDirty(); }
backup(token: CancellationToken): Promise<IWorkingCopyBackup> { return input.backup(token); }
save(options?: ISaveOptions): Promise<boolean> { return input.save(0, options).then(editor => !!editor); }
revert(options?: IRevertOptions): Promise<void> { return input.revert(0, options); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { IEditorFactoryRegistry, EditorExtensions } from 'vs/workbench/common/editor';
import { IEditorFactoryRegistry, EditorExtensions, EditorInputCapabilities } from 'vs/workbench/common/editor';
import { URI } from 'vs/base/common/uri';
import { workbenchInstantiationService, TestFileEditorInput, registerTestEditor, TestEditorPart, createEditorPart, registerTestSideBySideEditor } from 'vs/workbench/test/browser/workbenchTestServices';
import { Registry } from 'vs/platform/registry/common/platform';
Expand Down Expand Up @@ -628,4 +628,35 @@ suite('EditorsObserver', function () {
assert.strictEqual(observer.hasEditor({ resource: input3.resource, typeId: input3.typeId, editorId: input3.editorId }), true);
assert.strictEqual(observer.hasEditor({ resource: input4.resource, typeId: input4.typeId, editorId: input4.editorId }), true);
});

test('observer does not close scratchpads', async () => {
const [part] = await createPart();
part.enforcePartOptions({ limit: { enabled: true, value: 3 } });

const storage = new TestStorageService();
const observer = disposables.add(new EditorsObserver(part, storage));

const rootGroup = part.activeGroup;

const input1 = new TestFileEditorInput(URI.parse('foo://bar1'), TEST_EDITOR_INPUT_ID);
input1.capabilities = EditorInputCapabilities.Untitled | EditorInputCapabilities.Scratchpad;
const input2 = new TestFileEditorInput(URI.parse('foo://bar2'), TEST_EDITOR_INPUT_ID);
const input3 = new TestFileEditorInput(URI.parse('foo://bar3'), TEST_EDITOR_INPUT_ID);
const input4 = new TestFileEditorInput(URI.parse('foo://bar4'), TEST_EDITOR_INPUT_ID);

await rootGroup.openEditor(input1, { pinned: true });
await rootGroup.openEditor(input2, { pinned: true });
await rootGroup.openEditor(input3, { pinned: true });
await rootGroup.openEditor(input4, { pinned: true });

assert.strictEqual(rootGroup.count, 3);
assert.strictEqual(rootGroup.contains(input1), true);
assert.strictEqual(rootGroup.contains(input2), false);
assert.strictEqual(rootGroup.contains(input3), true);
assert.strictEqual(rootGroup.contains(input4), true);
assert.strictEqual(observer.hasEditor({ resource: input1.resource, typeId: input1.typeId, editorId: input1.editorId }), true);
assert.strictEqual(observer.hasEditor({ resource: input2.resource, typeId: input2.typeId, editorId: input2.editorId }), false);
assert.strictEqual(observer.hasEditor({ resource: input3.resource, typeId: input3.typeId, editorId: input3.editorId }), true);
assert.strictEqual(observer.hasEditor({ resource: input4.resource, typeId: input4.typeId, editorId: input4.editorId }), true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,10 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
return this.dirty;
}

isModified(): boolean {
return this.isDirty();
}

setDirty(dirty: boolean): void {
if (!this.isResolved()) {
return; // only resolved models can be marked dirty
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ suite('Files - TextFileEditorModel', () => {
model.updateTextEditorModel(createTextBufferFactory('bar'));
assert.ok(getLastModifiedTime(model) <= Date.now());
assert.ok(model.hasState(TextFileEditorModelState.DIRTY));
assert.ok(model.isModified());

assert.strictEqual(accessor.workingCopyService.dirtyCount, 1);
assert.strictEqual(accessor.workingCopyService.isDirty(model.resource, model.typeId), true);
Expand All @@ -123,6 +124,7 @@ suite('Files - TextFileEditorModel', () => {

assert.ok(model.hasState(TextFileEditorModelState.SAVED));
assert.ok(!model.isDirty());
assert.ok(!model.isModified());
assert.ok(savedEvent);
assert.ok((savedEvent as ITextFileEditorModelSaveEvent).stat);
assert.strictEqual((savedEvent as ITextFileEditorModelSaveEvent).reason, SaveReason.AUTO);
Expand Down Expand Up @@ -182,6 +184,7 @@ suite('Files - TextFileEditorModel', () => {

assert.ok(model.hasState(TextFileEditorModelState.ERROR));
assert.ok(model.isDirty());
assert.ok(model.isModified());
assert.ok(saveErrorEvent);

assert.strictEqual(accessor.workingCopyService.dirtyCount, 1);
Expand Down Expand Up @@ -238,6 +241,7 @@ suite('Files - TextFileEditorModel', () => {

assert.ok(model.hasState(TextFileEditorModelState.ERROR));
assert.ok(model.isDirty());
assert.ok(model.isModified());
assert.ok(saveErrorEvent);

assert.strictEqual(accessor.workingCopyService.dirtyCount, 1);
Expand Down Expand Up @@ -268,6 +272,7 @@ suite('Files - TextFileEditorModel', () => {

assert.ok(model.hasState(TextFileEditorModelState.CONFLICT));
assert.ok(model.isDirty());
assert.ok(model.isModified());
assert.ok(saveErrorEvent);

assert.strictEqual(accessor.workingCopyService.dirtyCount, 1);
Expand Down Expand Up @@ -457,6 +462,7 @@ suite('Files - TextFileEditorModel', () => {
await model.resolve();
model.updateTextEditorModel(createTextBufferFactory('foo'));
assert.ok(model.isDirty());
assert.ok(model.isModified());

assert.strictEqual(accessor.workingCopyService.dirtyCount, 1);
assert.strictEqual(accessor.workingCopyService.isDirty(model.resource, model.typeId), true);
Expand All @@ -471,6 +477,7 @@ suite('Files - TextFileEditorModel', () => {
model = accessor.workingCopyService.get(model) as TextFileEditorModel;

assert.strictEqual(model.isDirty(), false);
assert.strictEqual(model.isModified(), false);
assert.strictEqual(eventCounter, 1);

assert.ok(workingCopyEvent);
Expand All @@ -497,12 +504,14 @@ suite('Files - TextFileEditorModel', () => {
await model.resolve();
model.updateTextEditorModel(createTextBufferFactory('foo'));
assert.ok(model.isDirty());
assert.ok(model.isModified());

assert.strictEqual(accessor.workingCopyService.dirtyCount, 1);
assert.strictEqual(accessor.workingCopyService.isDirty(model.resource, model.typeId), true);

await model.revert({ soft: true });
assert.strictEqual(model.isDirty(), false);
assert.strictEqual(model.isModified(), false);
assert.strictEqual(model.textEditorModel!.getValue(), 'foo');
assert.strictEqual(eventCounter, 1);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ export class UntitledTextEditorModel extends BaseTextEditorModel implements IUnt
return this.dirty;
}

isModified(): boolean {
return this.isDirty();
}

private setDirty(dirty: boolean): void {
if (this.dirty === dirty) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,27 @@ export class BrowserWorkingCopyBackupTracker extends WorkingCopyBackupTracker im
protected onFinalBeforeShutdown(reason: ShutdownReason): boolean {

// Web: we cannot perform long running in the shutdown phase
// As such we need to check sync if there are any dirty working
// As such we need to check sync if there are any modified working
// copies that have not been backed up yet and then prevent the
// shutdown if that is the case.

const dirtyWorkingCopies = this.workingCopyService.dirtyWorkingCopies;
if (!dirtyWorkingCopies.length) {
return false; // no dirty: no veto
const modifiedWorkingCopies = this.workingCopyService.modifiedWorkingCopies;
if (!modifiedWorkingCopies.length) {
return false; // nothing modified: no veto
}

if (!this.filesConfigurationService.isHotExitEnabled) {
return true; // dirty without backup: veto
return true; // modified without backup: veto
}

for (const dirtyWorkingCopy of dirtyWorkingCopies) {
if (!this.workingCopyBackupService.hasBackupSync(dirtyWorkingCopy, this.getContentVersion(dirtyWorkingCopy))) {
for (const modifiedWorkingCopy of modifiedWorkingCopies) {
if (!this.workingCopyBackupService.hasBackupSync(modifiedWorkingCopy, this.getContentVersion(modifiedWorkingCopy))) {
this.logService.warn('Unload veto: pending backups');

return true; // dirty without backup: veto
return true; // modified without backup: veto
}
}

return false; // dirty with backups: no veto
return false; // modified and backed up: no veto
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ export abstract class ResourceWorkingCopy extends Disposable implements IResourc

//#endregion

//#region Modified Tracking

isModified(): boolean {
return this.isDirty();
}

//#endregion

//#region Abstract

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ export interface IUntitledFileWorkingCopyModelContentChangedEvent {

/**
* Flag that indicates that the content change should
* clear the dirty flag, e.g. because the contents are
* clear the dirty/modified flags, e.g. because the contents are
* back to being empty or back to an initial state that
* should not be considered as dirty.
* should not be considered as modified.
*/
readonly isInitial: boolean;
}
Expand Down Expand Up @@ -92,7 +92,7 @@ export interface IUntitledFileWorkingCopyInitialContents {

export class UntitledFileWorkingCopy<M extends IUntitledFileWorkingCopyModel> extends Disposable implements IUntitledFileWorkingCopy<M> {

readonly capabilities = WorkingCopyCapabilities.Untitled;
readonly capabilities = this.isScratchpad ? WorkingCopyCapabilities.Untitled | WorkingCopyCapabilities.Scratchpad : WorkingCopyCapabilities.Untitled;

private _model: M | undefined = undefined;
get model(): M | undefined { return this._model; }
Expand Down Expand Up @@ -121,6 +121,7 @@ export class UntitledFileWorkingCopy<M extends IUntitledFileWorkingCopyModel> ex
readonly resource: URI,
readonly name: string,
readonly hasAssociatedFilePath: boolean,
private readonly isScratchpad: boolean,
private readonly initialContents: IUntitledFileWorkingCopyInitialContents | undefined,
private readonly modelFactory: IUntitledFileWorkingCopyModelFactory<M>,
private readonly saveDelegate: IUntitledFileWorkingCopySaveDelegate<M>,
Expand All @@ -136,19 +137,25 @@ export class UntitledFileWorkingCopy<M extends IUntitledFileWorkingCopyModel> ex

//#region Dirty

private dirty = this.hasAssociatedFilePath || Boolean(this.initialContents && this.initialContents.markDirty !== false);
private modified = this.hasAssociatedFilePath || Boolean(this.initialContents && this.initialContents.markDirty !== false);

isDirty(): boolean {
return this.dirty;
return this.modified && !this.isScratchpad; // Scratchpad working copies are never dirty
}

private setDirty(dirty: boolean): void {
if (this.dirty === dirty) {
isModified(): boolean {
return this.modified;
}

private setModified(modified: boolean): void {
if (this.modified === modified) {
return;
}

this.dirty = dirty;
this._onDidChangeDirty.fire();
this.modified = modified;
if (!this.isScratchpad) {
this._onDidChangeDirty.fire();
}
}

//#endregion
Expand Down Expand Up @@ -189,8 +196,8 @@ export class UntitledFileWorkingCopy<M extends IUntitledFileWorkingCopyModel> ex
// Create model
await this.doCreateModel(untitledContents);

// Untitled associated to file path are dirty right away as well as untitled with content
this.setDirty(this.hasAssociatedFilePath || !!backup || Boolean(this.initialContents && this.initialContents.markDirty !== false));
// Untitled associated to file path are modified right away as well as untitled with content
this.setModified(this.hasAssociatedFilePath || !!backup || Boolean(this.initialContents && this.initialContents.markDirty !== false));

// If we have initial contents, make sure to emit this
// as the appropriate events to the outside.
Expand Down Expand Up @@ -220,16 +227,16 @@ export class UntitledFileWorkingCopy<M extends IUntitledFileWorkingCopyModel> ex

private onModelContentChanged(e: IUntitledFileWorkingCopyModelContentChangedEvent): void {

// Mark the untitled file working copy as non-dirty once its
// Mark the untitled file working copy as non-modified once its
// in case provided by the change event and in case we do not
// have an associated path set
if (!this.hasAssociatedFilePath && e.isInitial) {
this.setDirty(false);
this.setModified(false);
}

// Turn dirty otherwise
// Turn modified otherwise
else {
this.setDirty(true);
this.setModified(true);
}

// Emit as general content change event
Expand Down Expand Up @@ -287,8 +294,8 @@ export class UntitledFileWorkingCopy<M extends IUntitledFileWorkingCopyModel> ex
async revert(): Promise<void> {
this.trace('revert()');

// No longer dirty
this.setDirty(false);
// No longer modified
this.setModified(false);

// Emit as event
this._onDidRevert.fire();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ export interface INewOrExistingUntitledFileWorkingCopyOptions extends INewUntitl
* Note: the resource will not be used unless the scheme is `untitled`.
*/
untitledResource: URI;

/**
* A flag that will prevent the working copy from appearing dirty in the UI
* and not show a confirmation dialog when closed with unsaved content.
*/
isScratchpad?: boolean;
}

type IInternalUntitledFileWorkingCopyOptions = INewUntitledFileWorkingCopyOptions & INewUntitledFileWorkingCopyWithAssociatedResourceOptions & INewOrExistingUntitledFileWorkingCopyOptions;
Expand Down Expand Up @@ -167,6 +173,7 @@ export class UntitledFileWorkingCopyManager<M extends IUntitledFileWorkingCopyMo
// Handle untitled resource
else if (options.untitledResource?.scheme === Schemas.untitled) {
massagedOptions.untitledResource = options.untitledResource;
massagedOptions.isScratchpad = options.isScratchpad;
}

// Take over initial value
Expand Down Expand Up @@ -199,6 +206,7 @@ export class UntitledFileWorkingCopyManager<M extends IUntitledFileWorkingCopyMo
untitledResource,
this.labelService.getUriBasenameLabel(untitledResource),
!!options.associatedResource,
!!options.isScratchpad,
options.contents,
this.modelFactory,
this.saveDelegate,
Expand Down
Loading

0 comments on commit c97098e

Please sign in to comment.