diff --git a/examples/api-tests/src/saveable.spec.js b/examples/api-tests/src/saveable.spec.js index 9bfdd43f16c8e..25c36c350a717 100644 --- a/examples/api-tests/src/saveable.spec.js +++ b/examples/api-tests/src/saveable.spec.js @@ -23,14 +23,17 @@ describe('Saveable', function () { const { EditorManager } = require('@theia/editor/lib/browser/editor-manager'); const { EditorWidget } = require('@theia/editor/lib/browser/editor-widget'); const { PreferenceService } = require('@theia/core/lib/browser/preferences/preference-service'); + const { PreferenceScope } = require('@theia/core/lib/browser/preferences/preference-scope'); const { Saveable, SaveableWidget } = require('@theia/core/lib/browser/saveable'); const { WorkspaceService } = require('@theia/workspace/lib/browser/workspace-service'); const { FileService } = require('@theia/filesystem/lib/browser/file-service'); const { FileResource } = require('@theia/filesystem/lib/browser/file-resource'); const { ETAG_DISABLED } = require('@theia/filesystem/lib/common/files'); const { MonacoEditor } = require('@theia/monaco/lib/browser/monaco-editor'); + const { MonacoEditorModel } = require('@theia/monaco/lib/browser/monaco-editor-model'); const { Deferred } = require('@theia/core/lib/common/promise-util'); const { Disposable, DisposableCollection } = require('@theia/core/lib/common/disposable'); + const { AbstractResourcePreferenceProvider } = require('@theia/preferences/lib/browser/abstract-resource-preference-provider'); const container = window.theia.container; const editorManager = container.get(EditorManager); @@ -62,6 +65,72 @@ describe('Saveable', function () { return Disposable.create(() => fileResource['shouldOverwrite'] = originalShouldOverwrite); } + /** + * @param {MonacoEditorModel} modelToSpyOn + * @returns {{restore: Disposable, record: {calls: number}}} + */ + function spyOnSave(modelToSpyOn) { + assert.isTrue(modelToSpyOn instanceof MonacoEditorModel); + const toRestore = modelToSpyOn.save; + const callable = toRestore.bind(modelToSpyOn); + const record = { calls: 0 }; + modelToSpyOn.save = () => { + record.calls++; + return callable(); + }; + const restore = Disposable.create(() => modelToSpyOn.save = toRestore); + return { restore, record }; + } + + /** @typedef {Object} PrefTest + * @property {{calls: number}} record + * @property {boolean[]} initialValues + * @property {EditorWidget & SaveableWidget} editorWidget + * @property {MonacoEditor} prefEditor + * @property {AbstractResourcePreferenceProvider} provider + */ + + /** + * @param {string[]} preferencesToModify + * @returns {Promise} + */ + async function setUpPrefsTest(preferencesToModify) { + const preferenceFile = preferences.getConfigUri(PreferenceScope.Folder, rootUri.toString()); + assert.isDefined(preferenceFile); + if (!preferenceFile) { return; } + + const provider = preferences + .preferenceProviders + .get(PreferenceScope.Folder) + .providers + .get(preferenceFile.toString()); + + assert.isDefined(provider); + assert.isTrue(provider instanceof AbstractResourcePreferenceProvider); + + await Promise.all(preferencesToModify.map(name => preferences.set(name, undefined, undefined, rootUri.toString()))); + + const editorWidget = /** @type {EditorWidget & SaveableWidget} */ + (await editorManager.open(preferenceFile, { mode: 'reveal' })); + const prefEditor = MonacoEditor.get(editorWidget); + assert.isDefined(prefEditor); + if (!prefEditor) { return; } + assert.isFalse(Saveable.isDirty(editorWidget), 'should NOT be dirty on open'); + const model = prefEditor?.document; + assert.isDefined(model); + + if (!model) { return; } + const { restore, record } = spyOnSave(model); + toTearDown.push(restore); + /** @type {boolean[]} */ + const initialValues = preferencesToModify.map(name => + /** @type {boolean | undefined} */ + !!preferences.get(name, undefined, rootUri.toString()) + ); + + return { record, initialValues, editorWidget, prefEditor, provider }; + } + const toTearDown = new DisposableCollection(); /** @type {string | undefined} */ @@ -485,4 +554,93 @@ describe('Saveable', function () { } }); + it('saves preference file when open and not dirty', async function () { + const prefName = 'editor.copyWithSyntaxHighlighting'; + const prefTest = await setUpPrefsTest([prefName]); + if (!prefTest) { return; } + const { record, initialValues: [initialCopyWithHighlightPref], editorWidget } = prefTest; + await preferences.set(prefName, !initialCopyWithHighlightPref, undefined, rootUri.toString()); + /** @type {boolean | undefined} */ + const newValue = preferences.get(prefName, undefined, rootUri.toString()); + assert.equal(newValue, !initialCopyWithHighlightPref); + assert.isFalse(Saveable.isDirty(editorWidget), "editor should not be dirty if it wasn't dirty before"); + assert.equal(1, record.calls, 'save should have been called one time.'); + }); + + it('saves once when many edits are made (editor open)', async function () { + const prefNames = ['editor.copyWithSyntaxHighlighting', 'debug.inlineValues']; + const prefTest = await setUpPrefsTest(prefNames); + if (!prefTest) { return; } + + const { record, initialValues, editorWidget } = prefTest; + + /** @type {Promise[]} */ + const attempts = []; + while (attempts.length < 250) { + prefNames.forEach((prefName, index) => { + const value = attempts.length % 2 === 0 ? !initialValues[index] : initialValues[index]; + attempts.push(preferences.set(prefName, value, undefined, rootUri.toString())); + }); + } + await Promise.all(attempts); + assert.isFalse(Saveable.isDirty(editorWidget), "editor should not be dirty if it wasn't dirty before"); + assert.equal(1, record.calls, 'save should have been called one time.'); + }); + + it('saves once when many edits are made (editor closed)', async function () { + const prefNames = ['editor.copyWithSyntaxHighlighting', 'debug.inlineValues']; + const prefTest = await setUpPrefsTest(prefNames); + if (!prefTest) { return; } + + const { record, initialValues, editorWidget } = prefTest; + await editorWidget.closeWithoutSaving(); + + /** @type {Promise[]} */ + const attempts = []; + while (attempts.length < 250) { + prefNames.forEach((prefName, index) => { + const value = attempts.length % 2 === 0 ? !initialValues[index] : initialValues[index]; + attempts.push(preferences.set(prefName, value, undefined, rootUri.toString())); + }); + } + await Promise.all(attempts); + assert.equal(1, record.calls, 'save should have been called one time.'); + }); + + it('displays the toast once no matter how many edits are queued', async function () { + const prefNames = ['editor.copyWithSyntaxHighlighting', 'debug.inlineValues']; + const prefTest = await setUpPrefsTest(prefNames); + if (!prefTest) { return; } + + const { record, initialValues, editorWidget, prefEditor, provider } = prefTest; + const initialContent = prefEditor.getControl().getValue(); + prefEditor.getControl().setValue('anything dirty will do.'); + prefEditor.getControl().setValue(initialContent); + assert.isTrue(Saveable.isDirty(editorWidget)); + + const toRestore = provider['handleDirtyEditor']; + const callable = provider['handleDirtyEditor'].bind(provider); + const dirtyEditorRecord = { calls: 0 }; + const spy = () => { + dirtyEditorRecord.calls++; + return callable(); + }; + provider['handleDirtyEditor'] = spy; + toTearDown.push(Disposable.create(() => provider['handleDirtyEditor'] = toRestore)); + /** @type {Promise[]} */ + const attempts = []; + while (attempts.length < 250) { + prefNames.forEach((prefName, index) => { + const value = attempts.length % 2 === 0 ? !initialValues[index] : initialValues[index]; + attempts.push(preferences.set(prefName, value, undefined, rootUri.toString())); + }); + } + assert.equal(0, record.calls, 'should not have been saved yet'); + await Saveable.save(editorWidget); // Simulate the user saving and retrying. + provider['dirtyEditorHandled'].resolve(); + await Promise.all(attempts); + assert.equal(record.calls, 2, 'should have saved twice'); + assert.equal(dirtyEditorRecord.calls, 1, 'should have displayed the toast once'); + assert.equal(provider['editQueue'].length, 0, 'there should be no pending edits'); + }); }); diff --git a/packages/monaco/src/browser/monaco-workspace.ts b/packages/monaco/src/browser/monaco-workspace.ts index 909148bfd5049..c27905384bbb5 100644 --- a/packages/monaco/src/browser/monaco-workspace.ts +++ b/packages/monaco/src/browser/monaco-workspace.ts @@ -192,14 +192,14 @@ export class MonacoWorkspace { * Applies given edits to the given model. * The model is saved if no editors is opened for it. */ - applyBackgroundEdit(model: MonacoEditorModel, editOperations: monaco.editor.IIdentifiedSingleEditOperation[]): Promise { + applyBackgroundEdit(model: MonacoEditorModel, editOperations: monaco.editor.IIdentifiedSingleEditOperation[], shouldSave: boolean = true): Promise { return this.suppressOpenIfDirty(model, async () => { const editor = MonacoEditor.findByDocument(this.editorManager, model)[0]; const cursorState = editor && editor.getControl().getSelections() || []; model.textEditorModel.pushStackElement(); model.textEditorModel.pushEditOperations(cursorState, editOperations, () => cursorState); model.textEditorModel.pushStackElement(); - if (!editor) { + if (!editor && shouldSave) { await model.save(); } }); diff --git a/packages/preferences/src/browser/abstract-resource-preference-provider.ts b/packages/preferences/src/browser/abstract-resource-preference-provider.ts index 976f124514dcd..6cb5baa38a3bd 100644 --- a/packages/preferences/src/browser/abstract-resource-preference-provider.ts +++ b/packages/preferences/src/browser/abstract-resource-preference-provider.ts @@ -29,13 +29,23 @@ import { MonacoTextModelService } from '@theia/monaco/lib/browser/monaco-text-mo import { MonacoEditorModel } from '@theia/monaco/lib/browser/monaco-editor-model'; import { MonacoWorkspace } from '@theia/monaco/lib/browser/monaco-workspace'; import { Deferred } from '@theia/core/lib/common/promise-util'; +import { EditorManager } from '@theia/editor/lib/browser'; + +interface EnqueuedPreferenceChange { + key: string, + value: any, + editResolver: Deferred>, +} @injectable() export abstract class AbstractResourcePreferenceProvider extends PreferenceProvider { protected preferences: { [key: string]: any } = {}; protected model: MonacoEditorModel | undefined; + protected editQueue: EnqueuedPreferenceChange[] = []; protected readonly loading = new Deferred(); + protected dirtyEditorHandled = new Deferred(); + protected pendingTransaction = new Deferred(); @inject(PreferenceService) protected readonly preferenceService: PreferenceService; @inject(MessageService) protected readonly messageService: MessageService; @@ -50,11 +60,16 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi @inject(MonacoWorkspace) protected readonly workspace: MonacoWorkspace; + @inject(EditorManager) + protected readonly editorManager: EditorManager; + @postConstruct() protected async init(): Promise { const uri = this.getUri(); this.toDispose.push(Disposable.create(() => this.loading.reject(new Error(`preference provider for '${uri}' was disposed`)))); this._ready.resolve(); + this.dirtyEditorHandled.resolve(); + this.pendingTransaction.resolve(); const reference = await this.textModelService.createModelReference(uri); if (this.toDispose.disposed) { @@ -110,12 +125,57 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi async setPreference(key: string, value: any, resourceUri?: string): Promise { await this.loading.promise; + // Wait for save of previous transaction + await this.pendingTransaction.promise; if (!this.model) { return false; } if (!this.contains(resourceUri)) { return false; } + const isFirstEditInQueue = !this.editQueue.length; + const mustHandleDirty = isFirstEditInQueue && this.model.dirty; + if (mustHandleDirty) { + this.dirtyEditorHandled = new Deferred(); + this.handleDirtyEditor(); + } + const editResolver = new Deferred>(); + this.editQueue.push({ key, value, editResolver }); + if (isFirstEditInQueue) { + this.doSetPreferences(); + } + return editResolver.promise; + } + + async doSetPreferences(): Promise { + const localPendingChanges = new Deferred(); + await this.dirtyEditorHandled.promise; + while (this.editQueue.length) { + const { key, value, editResolver } = this.editQueue.shift()!; + const result = await this.doSetPreference(key, value) ? localPendingChanges.promise : false; + editResolver.resolve(result); + } + let success = true; + // Defer new actions until transaction complete. + // Prevents the dirty-editor toast from appearing if save cycle in progress. + this.pendingTransaction = new Deferred(); + try { + if (this.model?.dirty) { + await this.model.save(); + // On save, the file change handler will set ._pendingChanges + success = await this.pendingChanges; + } + } catch { + success = false; + } + this.pendingTransaction.resolve(); + localPendingChanges.resolve(success); + } + + protected async doSetPreference(key: string, value: any): Promise { + if (!this.model) { + return false; + } const path = this.getPath(key); if (!path) { return false; @@ -151,8 +211,8 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi forceMoveMarkers: false }); } - await this.workspace.applyBackgroundEdit(this.model, editOperations); - return await this.pendingChanges; + await this.workspace.applyBackgroundEdit(this.model, editOperations, false); + return true; } catch (e) { const message = `Failed to update the value of '${key}' in '${this.getUri()}'.`; this.messageService.error(`${message} Please check if it is corrupted.`); @@ -161,6 +221,25 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi } } + protected async handleDirtyEditor(): Promise { + const msg = await this.messageService.error( + 'Unable to write preference change because the settings file is dirty. Please save the file and try again.', + 'Save and Retry', 'Open File'); + + if (this.model) { + if (msg === 'Open File') { + this.editorManager.open(new URI(this.model.uri)); + } else if (msg === 'Save and Retry') { + await this.model.save(); + } + } + if (msg !== 'Save and Retry') { + this.editQueue.forEach(({ editResolver }) => editResolver.resolve(false)); + this.editQueue = []; + } + this.dirtyEditorHandled.resolve(); + } + protected getPath(preferenceName: string): string[] | undefined { return [preferenceName]; } diff --git a/packages/preferences/src/browser/views/components/preference-array-input.tsx b/packages/preferences/src/browser/views/components/preference-array-input.tsx index b937ada0a50b7..e72b3eb5b61d7 100644 --- a/packages/preferences/src/browser/views/components/preference-array-input.tsx +++ b/packages/preferences/src/browser/views/components/preference-array-input.tsx @@ -20,7 +20,7 @@ import { Preference } from '../../util/preference-types'; interface PreferenceArrayInputProps { preferenceDisplayNode: Preference.NodeWithValueInSingleScope; - setPreference(preferenceName: string, preferenceValue: string[]): void; + setPreference(preferenceName: string, preferenceValue: string[]): Promise; } export const PreferenceArrayInput: React.FC = ({ preferenceDisplayNode, setPreference }) => { diff --git a/packages/preferences/src/browser/views/components/preference-boolean-input.tsx b/packages/preferences/src/browser/views/components/preference-boolean-input.tsx index ba55d96e271d9..856f3d618e215 100644 --- a/packages/preferences/src/browser/views/components/preference-boolean-input.tsx +++ b/packages/preferences/src/browser/views/components/preference-boolean-input.tsx @@ -19,7 +19,7 @@ import { Preference } from '../../util/preference-types'; interface PreferenceBooleanInputProps { preferenceDisplayNode: Preference.NodeWithValueInSingleScope; - setPreference: (preferenceName: string, preferenceValue: boolean) => void; + setPreference: (preferenceName: string, preferenceValue: boolean) => Promise; } export const PreferenceBooleanInput: React.FC = ({ preferenceDisplayNode, setPreference }) => { @@ -34,10 +34,15 @@ export const PreferenceBooleanInput: React.FC = ({ setChecked(!!value); }, [value]); - const setValue = React.useCallback((e: React.ChangeEvent) => { + const setValue = React.useCallback(async (e: React.ChangeEvent) => { setChecked(!checked); - setPreference(id, e.target.checked); - }, [checked]); + const newValue = e.target.checked; + try { + await setPreference(id, newValue); + } catch { + setChecked(!!value); + } + }, [checked, value]); return (