mirror of
https://github.com/pybricks/pybricks-code.git
synced 2026-09-12 17:45:22 +00:00
editor: don't keep file open when editing
Since open takes a lock on the file, it prevented other file operations.
This commit is contained in:
@@ -3,7 +3,6 @@
|
||||
|
||||
import { mock } from 'jest-mock-extended';
|
||||
import { monaco } from 'react-monaco-editor';
|
||||
import { FD } from '../fileStorage/actions';
|
||||
import { ActiveFileHistoryManager, OpenFileInfo, OpenFileManager } from './lib';
|
||||
|
||||
afterEach(() => {
|
||||
@@ -98,11 +97,10 @@ describe('OpenFileManager', () => {
|
||||
|
||||
const model = mock<monaco.editor.ITextModel>();
|
||||
|
||||
manager.add('test.file', 0 as FD, model, null);
|
||||
manager.add('test.file', model, null);
|
||||
|
||||
expect(manager.has('test.file')).toBeTruthy();
|
||||
expect(manager.get('test.file')).toEqual(<OpenFileInfo>{
|
||||
fd: 0 as FD,
|
||||
model,
|
||||
viewState: null,
|
||||
});
|
||||
@@ -122,7 +120,7 @@ describe('OpenFileManager', () => {
|
||||
const model = mock<monaco.editor.ITextModel>();
|
||||
const viewState = mock<monaco.editor.ICodeEditorViewState>();
|
||||
|
||||
manager.add('test.file', 0 as FD, model, viewState);
|
||||
manager.add('test.file', model, viewState);
|
||||
|
||||
expect(manager.get('test.file')).toHaveProperty('viewState', viewState);
|
||||
|
||||
|
||||
+1
-5
@@ -3,7 +3,6 @@
|
||||
|
||||
import dexieObservable from 'dexie-observable';
|
||||
import { monaco } from 'react-monaco-editor';
|
||||
import { FD } from '../fileStorage/actions';
|
||||
|
||||
// HACK: Using window.name to detect page reloads vs. tab duplication.
|
||||
// window.name will persist across page reloads but will be set back to ''
|
||||
@@ -129,8 +128,6 @@ export class ActiveFileHistoryManager {
|
||||
}
|
||||
|
||||
export type OpenFileInfo = {
|
||||
/** The file descriptor. */
|
||||
readonly fd: FD;
|
||||
/** The model. */
|
||||
readonly model: monaco.editor.ITextModel;
|
||||
/** The view state. */
|
||||
@@ -142,7 +139,6 @@ export class OpenFileManager {
|
||||
|
||||
public add(
|
||||
fileName: string,
|
||||
fd: FD,
|
||||
model: monaco.editor.ITextModel,
|
||||
viewState: monaco.editor.ICodeEditorViewState | null,
|
||||
): void {
|
||||
@@ -151,7 +147,7 @@ export class OpenFileManager {
|
||||
throw new Error(`bug: key '${fileName}' already exists in the mpa`);
|
||||
}
|
||||
|
||||
this.map.set(fileName, { fd, model, viewState });
|
||||
this.map.set(fileName, { model, viewState });
|
||||
}
|
||||
|
||||
public remove(fileName: string): void {
|
||||
|
||||
+32
-72
@@ -5,16 +5,10 @@ import { mock } from 'jest-mock-extended';
|
||||
import { monaco } from 'react-monaco-editor';
|
||||
import { AsyncSaga } from '../../test';
|
||||
import {
|
||||
FD,
|
||||
fileStorageClose,
|
||||
fileStorageDidClose,
|
||||
fileStorageDidFailToOpen,
|
||||
fileStorageDidFailToRead,
|
||||
fileStorageDidFailToReadFile,
|
||||
fileStorageDidInitialize,
|
||||
fileStorageDidOpen,
|
||||
fileStorageDidRead,
|
||||
fileStorageOpen,
|
||||
fileStorageRead,
|
||||
fileStorageDidReadFile,
|
||||
fileStorageReadFile,
|
||||
} from '../fileStorage/actions';
|
||||
import {
|
||||
editorActivateFile,
|
||||
@@ -82,14 +76,14 @@ describe('per-editor sagas', () => {
|
||||
saga.put(editorOpenFile('test.file'));
|
||||
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
fileStorageOpen('test.file', 'w', false),
|
||||
fileStorageReadFile('test.file'),
|
||||
);
|
||||
});
|
||||
|
||||
it('should propagate error from fileStorageOpen', async () => {
|
||||
it('should propagate error from fileStorageReadFile', async () => {
|
||||
const testError = new Error('test error');
|
||||
|
||||
saga.put(fileStorageDidFailToOpen('test.file', testError));
|
||||
saga.put(fileStorageDidFailToReadFile('test.file', testError));
|
||||
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
editorDidFailToOpenFile('test.file', testError),
|
||||
@@ -98,82 +92,48 @@ describe('per-editor sagas', () => {
|
||||
expect(OpenFileManager.prototype.add).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('open succeeded', () => {
|
||||
describe('read succeeded', () => {
|
||||
let model: monaco.editor.ITextModel;
|
||||
|
||||
beforeEach(async () => {
|
||||
saga.put(fileStorageDidOpen('test.file', 0 as FD));
|
||||
await expect(saga.take()).resolves.toEqual(fileStorageRead(0 as FD));
|
||||
});
|
||||
monaco.editor.onDidCreateModel((m) => (model = m));
|
||||
|
||||
it('should propagate error from fileStorageRead', async () => {
|
||||
const testError = new Error('test error');
|
||||
saga.put(fileStorageDidReadFile('test.file', ''));
|
||||
|
||||
saga.put(fileStorageDidFailToRead(0 as FD, testError));
|
||||
|
||||
// file handle should be closed before editorDidFailToOpenFile
|
||||
await expect(saga.take()).resolves.toEqual(fileStorageClose(0 as FD));
|
||||
saga.put(fileStorageDidClose(0 as FD));
|
||||
expect(model).toBeDefined();
|
||||
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
editorDidFailToOpenFile('test.file', testError),
|
||||
editorDidOpenFile('test.file'),
|
||||
);
|
||||
|
||||
expect(OpenFileManager.prototype.add).not.toHaveBeenCalled();
|
||||
expect(OpenFileManager.prototype.add).toHaveBeenCalled();
|
||||
expect(OpenFileManager.prototype.remove).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('read succeeded', () => {
|
||||
let model: monaco.editor.ITextModel;
|
||||
it('should close file if task is canceled', async () => {
|
||||
jest.spyOn(model, 'dispose');
|
||||
|
||||
beforeEach(async () => {
|
||||
monaco.editor.onDidCreateModel((m) => (model = m));
|
||||
saga.cancel();
|
||||
|
||||
saga.put(fileStorageDidRead(0 as FD, ''));
|
||||
// model should be disposed before fileStorageClose
|
||||
expect(model.dispose).toHaveBeenCalled();
|
||||
expect(OpenFileManager.prototype.remove).toHaveBeenCalled();
|
||||
|
||||
expect(model).toBeDefined();
|
||||
// editorDidCloseFile is not called since we did not put editorCloseFile
|
||||
});
|
||||
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
editorDidOpenFile('test.file'),
|
||||
);
|
||||
it('should close when requested', async () => {
|
||||
jest.spyOn(model, 'dispose');
|
||||
|
||||
expect(OpenFileManager.prototype.add).toHaveBeenCalled();
|
||||
expect(OpenFileManager.prototype.remove).not.toHaveBeenCalled();
|
||||
});
|
||||
saga.put(editorCloseFile('test.file'));
|
||||
|
||||
it('should close file if task is canceled', async () => {
|
||||
jest.spyOn(model, 'dispose');
|
||||
// model should be disposed before fileStorageClose
|
||||
expect(model.dispose).toHaveBeenCalled();
|
||||
expect(OpenFileManager.prototype.remove).toHaveBeenCalled();
|
||||
|
||||
saga.cancel();
|
||||
|
||||
// model should be disposed before fileStorageClose
|
||||
expect(model.dispose).toHaveBeenCalled();
|
||||
expect(OpenFileManager.prototype.remove).toHaveBeenCalled();
|
||||
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
fileStorageClose(0 as FD),
|
||||
);
|
||||
saga.put(fileStorageDidClose(0 as FD));
|
||||
|
||||
// editorDidCloseFile is not called since we did not put editorCloseFile
|
||||
});
|
||||
|
||||
it('should close when requested', async () => {
|
||||
jest.spyOn(model, 'dispose');
|
||||
|
||||
saga.put(editorCloseFile('test.file'));
|
||||
|
||||
// model should be disposed before fileStorageClose
|
||||
expect(model.dispose).toHaveBeenCalled();
|
||||
expect(OpenFileManager.prototype.remove).toHaveBeenCalled();
|
||||
|
||||
// file handle should be closed before editorDidCloseFile
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
fileStorageClose(0 as FD),
|
||||
);
|
||||
saga.put(fileStorageDidClose(0 as FD));
|
||||
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
editorDidCloseFile('test.file'),
|
||||
);
|
||||
});
|
||||
await expect(saga.take()).resolves.toEqual(
|
||||
editorDidCloseFile('test.file'),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+11
-30
@@ -14,15 +14,10 @@ import {
|
||||
takeEvery,
|
||||
} from 'typed-redux-saga/macro';
|
||||
import {
|
||||
fileStorageClose,
|
||||
fileStorageDidClose,
|
||||
fileStorageDidFailToOpen,
|
||||
fileStorageDidFailToRead,
|
||||
fileStorageDidFailToReadFile,
|
||||
fileStorageDidInitialize,
|
||||
fileStorageDidOpen,
|
||||
fileStorageDidRead,
|
||||
fileStorageOpen,
|
||||
fileStorageRead,
|
||||
fileStorageDidReadFile,
|
||||
fileStorageReadFile,
|
||||
} from '../fileStorage/actions';
|
||||
import { RootState } from '../reducers';
|
||||
import { defined, ensureError } from '../utils';
|
||||
@@ -78,30 +73,19 @@ function* handleEditorOpenFile(
|
||||
let closeRequested = false;
|
||||
|
||||
try {
|
||||
yield* put(fileStorageOpen(action.fileName, 'w', false));
|
||||
|
||||
const { didOpen, didFailToOpen } = yield* race({
|
||||
didOpen: take(fileStorageDidOpen.when((a) => a.path === action.fileName)),
|
||||
didFailToOpen: take(
|
||||
fileStorageDidFailToOpen.when((a) => a.path === action.fileName),
|
||||
),
|
||||
});
|
||||
|
||||
if (didFailToOpen) {
|
||||
throw didFailToOpen.error;
|
||||
}
|
||||
|
||||
defined(didOpen);
|
||||
|
||||
const defer: Array<() => void> = [];
|
||||
|
||||
try {
|
||||
yield* put(fileStorageRead(didOpen.fd));
|
||||
yield* put(fileStorageReadFile(action.fileName));
|
||||
|
||||
const { didRead, didFailToRead } = yield* race({
|
||||
didRead: take(fileStorageDidRead.when((a) => a.fd === didOpen.fd)),
|
||||
didRead: take(
|
||||
fileStorageDidReadFile.when((a) => a.path === action.fileName),
|
||||
),
|
||||
didFailToRead: take(
|
||||
fileStorageDidFailToRead.when((a) => a.fd === didOpen.fd),
|
||||
fileStorageDidFailToReadFile.when(
|
||||
(a) => a.path === action.fileName,
|
||||
),
|
||||
),
|
||||
});
|
||||
|
||||
@@ -120,7 +104,7 @@ function* handleEditorOpenFile(
|
||||
|
||||
// TODO: get viewState from fileStorage
|
||||
|
||||
openFiles.add(action.fileName, didOpen.fd, model, null);
|
||||
openFiles.add(action.fileName, model, null);
|
||||
defer.push(() => openFiles.remove(action.fileName));
|
||||
|
||||
yield* put(editorDidOpenFile(action.fileName));
|
||||
@@ -133,9 +117,6 @@ function* handleEditorOpenFile(
|
||||
callback();
|
||||
}
|
||||
|
||||
yield* put(fileStorageClose(didOpen.fd));
|
||||
yield* take(fileStorageDidClose.when((a) => a.fd === didOpen.fd));
|
||||
|
||||
// only send the did close action if the corresponding action requested it
|
||||
if (closeRequested) {
|
||||
yield* put(editorDidCloseFile(action.fileName));
|
||||
|
||||
Reference in New Issue
Block a user