Skip to content

Commit fdf70cb

Browse files
sandy081sandy081
committed
microsoft#100346 fix opening diffs and accept preview
1 parent 28f3fa9 commit fdf70cb

13 files changed

Lines changed: 229 additions & 161 deletions

File tree

‎src/vs/platform/userDataSync/common/abstractSynchronizer.ts‎

Lines changed: 62 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ export interface IMergableResourcePreview extends IBaseResourcePreview {
5757
readonly remoteContent: string | null;
5858
readonly localContent: string | null;
5959
readonly previewContent: string | null;
60+
readonly acceptedContent: string | null;
6061
readonly hasConflicts: boolean;
61-
mergeState: MergeState;
6262
}
6363

6464
export type IResourcePreview = Omit<IMergableResourcePreview, 'mergeState'>;
@@ -385,48 +385,58 @@ export abstract class AbstractSynchroniser extends Disposable {
385385
}
386386

387387
async accept(resource: URI, content: string): Promise<ISyncResourcePreview | null> {
388-
if (!this.syncPreviewPromise) {
389-
return null;
390-
}
391-
392-
let preview = await this.syncPreviewPromise;
393-
this.syncPreviewPromise = createCancelablePromise(token => this.updateSyncResourcePreviewContent(preview, resource, content, token));
394-
395-
preview = await this.syncPreviewPromise;
396-
this.updateConflicts(preview.resourcePreviews);
397-
if (preview.resourcePreviews.some(({ mergeState }) => mergeState === MergeState.Conflict)) {
398-
this.setStatus(SyncStatus.HasConflicts);
399-
} else {
400-
this.setStatus(SyncStatus.Syncing);
401-
}
402-
403-
return preview;
388+
await this.updateSyncResourcePreview(resource, async (resourcePreview) => {
389+
const updatedResourcePreview = await this.updateResourcePreview(resourcePreview, resource, content);
390+
return {
391+
...updatedResourcePreview,
392+
mergeState: MergeState.Accepted
393+
};
394+
});
395+
return this.syncPreviewPromise;
404396
}
405397

406398
async merge(resource: URI): Promise<ISyncResourcePreview | null> {
407-
await this.changeMergeState(resource, (resourcePreview) => resourcePreview.hasConflicts ? MergeState.Conflict : MergeState.Accepted);
399+
await this.updateSyncResourcePreview(resource, async (resourcePreview) => ({
400+
...resourcePreview,
401+
mergeState: resourcePreview.hasConflicts ? MergeState.Conflict : MergeState.Accepted
402+
}));
408403
return this.syncPreviewPromise;
409404
}
410405

411406
async discard(resource: URI): Promise<ISyncResourcePreview | null> {
412-
await this.changeMergeState(resource, () => MergeState.Preview);
407+
await this.updateSyncResourcePreview(resource, async (resourcePreview) => {
408+
await this.fileService.writeFile(resourcePreview.previewResource, VSBuffer.fromString(resourcePreview.previewContent || ''));
409+
const updatedPreview = await this.updateResourcePreview(resourcePreview, resourcePreview.previewResource, resourcePreview.previewContent || '');
410+
return {
411+
...updatedPreview,
412+
mergeState: MergeState.Preview
413+
};
414+
});
413415
return this.syncPreviewPromise;
414416
}
415417

416-
private async changeMergeState(resource: URI, mergeState: (resourcePreview: IMergableResourcePreview) => MergeState): Promise<void> {
418+
private async updateSyncResourcePreview(resource: URI, updateResourcePreview: (resourcePreview: IMergableResourcePreview) => Promise<IMergableResourcePreview>): Promise<void> {
417419
if (!this.syncPreviewPromise) {
418420
return;
419421
}
420422

421-
const preview = await this.syncPreviewPromise;
422-
const resourcePreview = preview.resourcePreviews.find(({ localResource, remoteResource, previewResource }) =>
423+
let preview = await this.syncPreviewPromise;
424+
const index = preview.resourcePreviews.findIndex(({ localResource, remoteResource, previewResource }) =>
423425
isEqual(localResource, resource) || isEqual(remoteResource, resource) || isEqual(previewResource, resource));
424-
if (!resourcePreview) {
426+
if (index === -1) {
425427
return;
426428
}
427429

428-
resourcePreview.mergeState = mergeState(resourcePreview);
430+
this.syncPreviewPromise = createCancelablePromise(async token => {
431+
const resourcePreviews = [...preview.resourcePreviews];
432+
resourcePreviews[index] = await updateResourcePreview(resourcePreviews[index]);
433+
return {
434+
...preview,
435+
resourcePreviews
436+
};
437+
});
429438

439+
preview = await this.syncPreviewPromise;
430440
this.updateConflicts(preview.resourcePreviews);
431441
if (preview.resourcePreviews.some(({ mergeState }) => mergeState === MergeState.Conflict)) {
432442
this.setStatus(SyncStatus.HasConflicts);
@@ -435,6 +445,13 @@ export abstract class AbstractSynchroniser extends Disposable {
435445
}
436446
}
437447

448+
protected async updateResourcePreview(resourcePreview: IResourcePreview, resource: URI, acceptedContent: string): Promise<IResourcePreview> {
449+
return {
450+
...resourcePreview,
451+
acceptedContent
452+
};
453+
}
454+
438455
private async doApply(force: boolean): Promise<SyncStatus> {
439456
if (!this.syncPreviewPromise) {
440457
return SyncStatus.Idle;
@@ -464,32 +481,6 @@ export abstract class AbstractSynchroniser extends Disposable {
464481
return SyncStatus.Idle;
465482
}
466483

467-
private async updateSyncResourcePreviewContent(preview: ISyncResourcePreview, resource: URI, previewContent: string, token: CancellationToken): Promise<ISyncResourcePreview> {
468-
const index = preview.resourcePreviews.findIndex(({ localResource, remoteResource, previewResource, localChange, remoteChange }) =>
469-
(localChange !== Change.None || remoteChange !== Change.None)
470-
&& (isEqual(localResource, resource) || isEqual(remoteResource, resource) || isEqual(previewResource, resource)));
471-
if (index !== -1) {
472-
const resourcePreviews = [...preview.resourcePreviews];
473-
const resourcePreview = await this.updateResourcePreviewContent(resourcePreviews[index], resource, previewContent, token);
474-
resourcePreviews[index] = { ...resourcePreview, mergeState: MergeState.Accepted };
475-
preview = {
476-
...preview,
477-
resourcePreviews
478-
};
479-
}
480-
return preview;
481-
}
482-
483-
protected async updateResourcePreviewContent(resourcePreview: IResourcePreview, resource: URI, previewContent: string, token: CancellationToken): Promise<IResourcePreview> {
484-
return {
485-
...resourcePreview,
486-
previewContent,
487-
hasConflicts: false,
488-
localChange: Change.Modified,
489-
remoteChange: Change.Modified,
490-
};
491-
}
492-
493484
private async clearPreviewFolder(): Promise<void> {
494485
try {
495486
await this.fileService.del(this.syncPreviewFolder, { recursive: true });
@@ -555,13 +546,13 @@ export abstract class AbstractSynchroniser extends Disposable {
555546
const syncPreview = this.syncPreviewPromise ? await this.syncPreviewPromise : null;
556547
if (syncPreview) {
557548
for (const resourcePreview of syncPreview.resourcePreviews) {
558-
if (resourcePreview.previewResource && isEqual(resourcePreview.previewResource, uri)) {
559-
return resourcePreview.previewContent || '';
549+
if (isEqual(resourcePreview.acceptedResource, uri)) {
550+
return resourcePreview.acceptedContent || '';
560551
}
561-
if (resourcePreview.remoteResource && isEqual(resourcePreview.remoteResource, uri)) {
552+
if (isEqual(resourcePreview.remoteResource, uri)) {
562553
return resourcePreview.remoteContent || '';
563554
}
564-
if (resourcePreview.localResource && isEqual(resourcePreview.localResource, uri)) {
555+
if (isEqual(resourcePreview.localResource, uri)) {
565556
return resourcePreview.localContent || '';
566557
}
567558
}
@@ -581,18 +572,25 @@ export abstract class AbstractSynchroniser extends Disposable {
581572

582573
// For preview, use remoteUserData if lastSyncUserData does not exists and last sync is from current machine
583574
const lastSyncUserDataForPreview = lastSyncUserData === null && isLastSyncFromCurrentMachine ? remoteUserData : lastSyncUserData;
584-
const resourcePreviews = await this.generateSyncPreview(remoteUserData, lastSyncUserDataForPreview, token);
585-
586-
// Mark merge
587-
const mergableResourcePreviews = resourcePreviews.map<IMergableResourcePreview>(r => ({
588-
...r,
589-
mergeState: r.localChange === Change.None && r.remoteChange === Change.None ? MergeState.Accepted /* Mark previews with no changes as merged */
590-
: apply ? (r.hasConflicts ? MergeState.Conflict : MergeState.Accepted)
591-
: MergeState.Preview
575+
const result = await this.generateSyncPreview(remoteUserData, lastSyncUserDataForPreview, token);
592576

593-
}));
577+
const resourcePreviews: IMergableResourcePreview[] = [];
578+
for (const resourcePreview of result) {
579+
if (token.isCancellationRequested) {
580+
break;
581+
}
582+
if (!apply) {
583+
await this.fileService.writeFile(resourcePreview.previewResource, VSBuffer.fromString(resourcePreview.previewContent || ''));
584+
}
585+
resourcePreviews.push({
586+
...resourcePreview,
587+
mergeState: resourcePreview.localChange === Change.None && resourcePreview.remoteChange === Change.None ? MergeState.Accepted /* Mark previews with no changes as merged */
588+
: apply ? (resourcePreview.hasConflicts ? MergeState.Conflict : MergeState.Accepted)
589+
: MergeState.Preview
590+
});
591+
}
594592

595-
return { remoteUserData, lastSyncUserData, resourcePreviews: mergableResourcePreviews, isLastSyncFromCurrentMachine };
593+
return { remoteUserData, lastSyncUserData, resourcePreviews, isLastSyncFromCurrentMachine };
596594
}
597595

598596
async getLastSyncUserData<T extends IRemoteUserData>(): Promise<T | null> {

‎src/vs/platform/userDataSync/common/extensionsSync.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
4747
protected readonly version: number = 3;
4848
protected isEnabled(): boolean { return super.isEnabled() && this.extensionGalleryService.isEnabled(); }
4949
private readonly localPreviewResource: URI = joinPath(this.syncPreviewFolder, 'extensions.json');
50-
private readonly remotePreviewResource: URI = this.localPreviewResource.with({ scheme: USER_DATA_SYNC_SCHEME });
50+
private readonly remotePreviewResource: URI = this.localPreviewResource.with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'remote' });
51+
private readonly acceptedPreviewResource: URI = this.localPreviewResource.with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'accepted' });
5152

5253
constructor(
5354
@IEnvironmentService environmentService: IEnvironmentService,
@@ -100,6 +101,8 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
100101
remoteContent: remoteExtensions ? this.format(remoteExtensions) : null,
101102
previewResource: this.localPreviewResource,
102103
previewContent: null,
104+
acceptedResource: this.acceptedPreviewResource,
105+
acceptedContent: null,
103106
added,
104107
removed,
105108
updated,
@@ -137,6 +140,8 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
137140
remoteContent: remoteExtensions ? this.format(remoteExtensions) : null,
138141
previewResource: this.localPreviewResource,
139142
previewContent: null,
143+
acceptedResource: this.acceptedPreviewResource,
144+
acceptedContent: null,
140145
added,
141146
removed,
142147
updated,
@@ -177,14 +182,14 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
177182
}
178183
}
179184

180-
protected async updateResourcePreviewContent(resourcePreview: IExtensionResourcePreview, resource: URI, previewContent: string, token: CancellationToken): Promise<IExtensionResourcePreview> {
185+
protected async updateResourcePreview(resourcePreview: IExtensionResourcePreview, resource: URI, acceptedContent: string): Promise<IExtensionResourcePreview> {
181186
if (isEqual(resource, ExtensionsSynchroniser.EXTENSIONS_DATA_URI)) {
182187
const remoteExtensions = resourcePreview.remoteContent ? JSON.parse(resourcePreview.remoteContent) : null;
183188
return this.getPushPreview(remoteExtensions);
184189
}
185190
return {
186191
...resourcePreview,
187-
previewContent,
192+
acceptedContent,
188193
hasConflicts: false,
189194
localChange: Change.Modified,
190195
remoteChange: Change.Modified,
@@ -199,6 +204,7 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
199204
const localContent = this.format(localExtensions);
200205
const remoteResource = this.remotePreviewResource;
201206
const previewResource = this.localPreviewResource;
207+
const acceptedResource = this.acceptedPreviewResource;
202208
const previewContent = null;
203209
if (remoteExtensions !== null) {
204210
const mergeResult = merge(localExtensions, remoteExtensions, localExtensions, [], ignoredExtensions);
@@ -210,6 +216,8 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
210216
remoteContent: this.format(remoteExtensions),
211217
previewResource,
212218
previewContent,
219+
acceptedResource,
220+
acceptedContent: previewContent,
213221
added,
214222
removed,
215223
updated,
@@ -228,6 +236,8 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
228236
remoteContent: null,
229237
previewResource,
230238
previewContent,
239+
acceptedResource,
240+
acceptedContent: previewContent,
231241
added: [], removed: [], updated: [], remote: null, localExtensions, skippedExtensions: [],
232242
localChange: Change.None,
233243
remoteChange: Change.None,
@@ -249,6 +259,8 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
249259
remoteContent: remoteExtensions ? this.format(remoteExtensions) : null,
250260
previewResource: this.localPreviewResource,
251261
previewContent: null,
262+
acceptedResource: this.acceptedPreviewResource,
263+
acceptedContent: null,
252264
added,
253265
removed,
254266
updated,
@@ -272,7 +284,7 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse
272284
return this.format(localExtensions);
273285
}
274286

275-
if (isEqual(this.remotePreviewResource, uri) || isEqual(this.localPreviewResource, uri)) {
287+
if (isEqual(this.remotePreviewResource, uri) || isEqual(this.acceptedPreviewResource, uri)) {
276288
return this.resolvePreviewContent(uri);
277289
}
278290

‎src/vs/platform/userDataSync/common/globalStateSync.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
4646
private static readonly GLOBAL_STATE_DATA_URI = URI.from({ scheme: USER_DATA_SYNC_SCHEME, authority: 'globalState', path: `/globalState.json` });
4747
protected readonly version: number = 1;
4848
private readonly localPreviewResource: URI = joinPath(this.syncPreviewFolder, 'globalState.json');
49-
private readonly remotePreviewResource: URI = this.localPreviewResource.with({ scheme: USER_DATA_SYNC_SCHEME });
49+
private readonly remotePreviewResource: URI = this.localPreviewResource.with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'remote' });
50+
private readonly acceptedPreviewResource: URI = this.localPreviewResource.with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'accepted' });
5051

5152
constructor(
5253
@IFileService fileService: IFileService,
@@ -99,6 +100,8 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
99100
remoteContent: remoteGlobalState ? this.format(remoteGlobalState) : null,
100101
previewResource: this.localPreviewResource,
101102
previewContent: null,
103+
acceptedResource: this.acceptedPreviewResource,
104+
acceptedContent: null,
102105
local,
103106
remote: syncGlobalState.storage,
104107
localUserData,
@@ -131,6 +134,8 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
131134
remoteContent: remoteGlobalState ? this.format(remoteGlobalState) : null,
132135
previewResource: this.localPreviewResource,
133136
previewContent: null,
137+
acceptedResource: this.acceptedPreviewResource,
138+
acceptedContent: null,
134139
local,
135140
remote,
136141
localUserData: localGloablState,
@@ -172,7 +177,7 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
172177
}
173178
}
174179

175-
protected async updateResourcePreviewContent(resourcePreview: IGlobalStateResourcePreview, resource: URI, previewContent: string, token: CancellationToken): Promise<IGlobalStateResourcePreview> {
180+
protected async updateResourcePreview(resourcePreview: IGlobalStateResourcePreview, resource: URI, acceptedContent: string): Promise<IGlobalStateResourcePreview> {
176181
if (GlobalStateSynchroniser.GLOBAL_STATE_DATA_URI, resource) {
177182
return this.getPushPreview(resourcePreview.remoteContent);
178183
}
@@ -188,6 +193,7 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
188193
const localContent = this.format(localGlobalState);
189194
const remoteResource = this.remotePreviewResource;
190195
const previewResource = this.localPreviewResource;
196+
const acceptedResource = this.acceptedPreviewResource;
191197
const previewContent = null;
192198
if (remoteContent !== null) {
193199
const remoteGlobalState: IGlobalState = JSON.parse(remoteContent);
@@ -200,6 +206,8 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
200206
remoteContent: this.format(remoteGlobalState),
201207
previewResource,
202208
previewContent,
209+
acceptedResource,
210+
acceptedContent: previewContent,
203211
local,
204212
remote,
205213
localUserData: localGlobalState,
@@ -216,6 +224,8 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
216224
remoteContent: null,
217225
previewResource,
218226
previewContent,
227+
acceptedResource,
228+
acceptedContent: previewContent,
219229
local: { added: {}, removed: [], updated: {} },
220230
remote: null,
221231
localUserData: localGlobalState,
@@ -237,6 +247,8 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
237247
remoteContent: remoteGlobalState ? this.format(remoteGlobalState) : null,
238248
previewResource: this.localPreviewResource,
239249
previewContent: null,
250+
acceptedResource: this.acceptedPreviewResource,
251+
acceptedContent: null,
240252
local: { added: {}, removed: [], updated: {} },
241253
remote: localUserData.storage,
242254
localUserData,
@@ -257,7 +269,7 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
257269
return this.format(localGlobalState);
258270
}
259271

260-
if (isEqual(this.remotePreviewResource, uri) || isEqual(this.localPreviewResource, uri)) {
272+
if (isEqual(this.remotePreviewResource, uri) || isEqual(this.acceptedPreviewResource, uri)) {
261273
return this.resolvePreviewContent(uri);
262274
}
263275

0 commit comments

Comments
 (0)