Skip to content

Commit

Permalink
Show readonly filename decoration after revert that also resets mtime (
Browse files Browse the repository at this point in the history
…fix #221014) (#221023)

* Show readonly filename decoration after revert

* Use a different approach that obeys 'increasing mtime' rule

* Also update locked flag

* Add tests
  • Loading branch information
gjsjohnmurray authored Jul 29, 2024
1 parent 0f9f2a0 commit 0c5af5d
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,11 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
this.lastResolvedFileStat = newFileStat;
}

// In all other cases update only the readonly and locked flags
else {
this.lastResolvedFileStat = { ...this.lastResolvedFileStat, readonly: newFileStat.readonly, locked: newFileStat.locked };
}

// Signal that the readonly state changed
if (this.isReadonly() !== oldReadonly) {
this._onDidChangeReadonly.fire();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { EncodingMode, TextFileEditorModelState, snapshotToString, isTextFileEdi
import { createFileEditorInput, workbenchInstantiationService, TestServiceAccessor, TestReadonlyTextFileEditorModel, getLastResolvedFileStat } from 'vs/workbench/test/browser/workbenchTestServices';
import { ensureNoDisposablesAreLeakedInTestSuite, toResource } from 'vs/base/test/common/utils';
import { TextFileEditorModelManager } from 'vs/workbench/services/textfile/common/textFileEditorModelManager';
import { FileOperationResult, FileOperationError } from 'vs/platform/files/common/files';
import { FileOperationResult, FileOperationError, NotModifiedSinceFileOperationError } from 'vs/platform/files/common/files';
import { DeferredPromise, timeout } from 'vs/base/common/async';
import { assertIsDefined } from 'vs/base/common/types';
import { createTextBufferFactory } from 'vs/editor/common/model/textModel';
Expand Down Expand Up @@ -608,6 +608,22 @@ suite('Files - TextFileEditorModel', () => {
assert.strictEqual(getLastModifiedTime(model), mtime);
});

test('stat.readonly and stat.locked can change when decreased mtime is ignored', async function () {
const model: TextFileEditorModel = disposables.add(instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8', undefined));

await model.resolve();

const stat = assertIsDefined(getLastResolvedFileStat(model));
accessor.textFileService.setReadStreamErrorOnce(new NotModifiedSinceFileOperationError('error', { ...stat, mtime: stat.mtime - 1, readonly: !stat.readonly, locked: !stat.locked }));

await model.resolve();

assert.ok(model);
assert.strictEqual(getLastModifiedTime(model), stat.mtime, 'mtime should not decrease');
assert.notStrictEqual(getLastResolvedFileStat(model)?.readonly, stat.readonly, 'readonly should have changed despite simultaneous attempt to decrease mtime');
assert.notStrictEqual(getLastResolvedFileStat(model)?.locked, stat.locked, 'locked should have changed despite simultaneous attempt to decrease mtime');
});

test('Resolve error is handled gracefully if model already exists', async function () {
const model: TextFileEditorModel = disposables.add(instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8', undefined));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,11 @@ export class StoredFileWorkingCopy<M extends IStoredFileWorkingCopyModel> extend
this.lastResolvedFileStat = newFileStat;
}

// In all other cases update only the readonly and locked flags
else {
this.lastResolvedFileStat = { ...this.lastResolvedFileStat, readonly: newFileStat.readonly, locked: newFileStat.locked };
}

// Signal that the readonly state changed
if (this.isReadonly() !== oldReadonly) {
this._onDidChangeReadonly.fire();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { StoredFileWorkingCopy, StoredFileWorkingCopyState, IStoredFileWorkingCo
import { bufferToStream, newWriteableBufferStream, streamToBuffer, VSBuffer, VSBufferReadableStream } from 'vs/base/common/buffer';
import { CancellationToken } from 'vs/base/common/cancellation';
import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
import { TestServiceAccessor, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
import { getLastResolvedFileStat, TestServiceAccessor, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { basename } from 'vs/base/common/resources';
import { FileChangesEvent, FileChangeType, FileOperationError, FileOperationResult, IFileStatWithMetadata, IWriteFileOptions, NotModifiedSinceFileOperationError } from 'vs/platform/files/common/files';
Expand All @@ -20,6 +20,7 @@ import { consumeReadable, consumeStream, isReadableStream } from 'vs/base/common
import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { SnapshotContext } from 'vs/workbench/services/workingCopy/common/fileWorkingCopy';
import { assertIsDefined } from 'vs/base/common/types';

export class TestStoredFileWorkingCopyModel extends Disposable implements IStoredFileWorkingCopyModel {

Expand Down Expand Up @@ -513,6 +514,23 @@ suite('StoredFileWorkingCopy', function () {
});
});

test('stat.readonly and stat.locked can change when decreased mtime is ignored', async function () {

await workingCopy.resolve();

const stat = assertIsDefined(getLastResolvedFileStat(workingCopy));
try {
accessor.fileService.readShouldThrowError = new NotModifiedSinceFileOperationError('error', { ...stat, mtime: stat.mtime - 1, readonly: !stat.readonly, locked: !stat.locked });
await workingCopy.resolve();
} finally {
accessor.fileService.readShouldThrowError = undefined;
}

assert.strictEqual(getLastResolvedFileStat(workingCopy)?.mtime, stat.mtime, 'mtime should not decrease');
assert.notStrictEqual(getLastResolvedFileStat(workingCopy)?.readonly, stat.readonly, 'readonly should have changed despite simultaneous attempt to decrease mtime');
assert.notStrictEqual(getLastResolvedFileStat(workingCopy)?.locked, stat.locked, 'locked should have changed despite simultaneous attempt to decrease mtime');
});

test('resolve (FILE_NOT_MODIFIED_SINCE can be handled for resolved working copies)', async () => {
await workingCopy.resolve();

Expand Down

0 comments on commit 0c5af5d

Please sign in to comment.