From 2d8ee9f7bdcfda78518f8bb91178edcb6a4aa115 Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Tue, 25 Feb 2025 18:47:01 +0100 Subject: [PATCH] Handle conflicts resolution end to end --- src/main.ts | 22 ++++-- src/sync-manager.ts | 88 +++++++++++++++------ src/views/conflicts-resolution/view.tsx | 101 ++++-------------------- 3 files changed, 93 insertions(+), 118 deletions(-) diff --git a/src/main.ts b/src/main.ts index ebf7bd4..3cfc4d4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,8 +1,13 @@ -import { EventRef, Plugin, Platform, WorkspaceLeaf } from "obsidian"; +import { + EventRef, + Plugin, + Platform, + WorkspaceLeaf, + normalizePath, +} from "obsidian"; import { GitHubSyncSettings, DEFAULT_SETTINGS } from "./settings/settings"; import GitHubSyncSettingsTab from "./settings/tab"; import SyncManager, { ConflictFile, ConflictResolution } from "./sync-manager"; -import { FileMetadata } from "./metadata-store"; import { OnboardingDialog } from "./views/onboarding/view"; import Logger from "./logger"; import { @@ -29,6 +34,12 @@ export default class GitHubSyncPlugin extends Plugin { conflictsResolver: ((resolutions: ConflictResolution[]) => void) | null = null; + // We keep track of the sync conflicts in here too in case the + // conflicts view must be rebuilt, or the user closes the view + // and it gets destroyed. + // By keeping them here we can recreate it easily. + private conflicts: ConflictFile[] = []; + async onUserEnable() { if (Platform.isMobile) { // TODO: Implement onboarding for mobile @@ -72,11 +83,11 @@ export default class GitHubSyncPlugin extends Plugin { this.registerView( CONFLICTS_RESOLUTION_VIEW_TYPE, - (leaf) => new ConflictsResolutionView(leaf, this), + (leaf) => new ConflictsResolutionView(leaf, this, this.conflicts), ); - this.addRibbonIcon("dice", "Activate view", async () => { + this.addRibbonIcon("merge", "Open sync conflicts resolution", async () => { await this.activateView(); - this.getConflictsView()?.setConflictFiles([]); + this.getConflictsView()?.setConflictFiles(this.conflicts); }); this.logger = new Logger(this.app.vault, this.settings.enableLogging); @@ -199,6 +210,7 @@ export default class GitHubSyncPlugin extends Plugin { } async onConflicts(conflicts: ConflictFile[]): Promise { + this.conflicts = conflicts; return await new Promise(async (resolve) => { this.conflictsResolver = resolve; await this.activateView(); diff --git a/src/sync-manager.ts b/src/sync-manager.ts index 00ead8d..b376416 100644 --- a/src/sync-manager.ts +++ b/src/sync-manager.ts @@ -302,39 +302,34 @@ export default class SyncManager { const remoteMetadata: Metadata = JSON.parse(atob(blob.content)); const conflicts = await this.findConflicts(remoteMetadata.files); - let conflictResolutions: SyncAction[] = []; - if (conflicts.length > 0) { - // TODO: Show conflicts to the user with a callback - // Wait for response - // Create new action to handle the conflict - // Add the new action to the list later on - console.log("Conflicts"); - console.log(conflicts); - await this.logger.warn("Found conflicts", conflicts); - (await this.onConflicts(conflicts)).forEach( - (resolution: boolean, index: number) => { - if (resolution) { - conflictResolutions.push({ - type: "download", - filePath: conflicts[index].remoteFile.path, - }); - } else { - conflictResolutions.push({ - type: "upload", - filePath: conflicts[index].localFile.path, - }); - } + // We treat every resolved conflict as an upload SyncAction, mainly cause + // the user has complete freedom on the edits they can apply to the conflicting files. + // So when a conflict is resolved we change the file locally and upload it. + // That solves the conflict. + let conflictActions: SyncAction[] = []; + // We keep track of the conflict resolutions cause we want to update the file + // locally only when we're sure the sync was successul. That happens after we + // commit the sync. + let conflictResolutions: ConflictResolution[] = []; + + if (conflicts.length > 0) { + await this.logger.warn("Found conflicts", conflicts); + // Here we block the sync process until the user has resolved all the conflicts + conflictResolutions = await this.onConflicts(conflicts); + conflictActions = conflictResolutions.map( + (resolution: ConflictResolution) => { + return { type: "upload", filePath: resolution.filename }; }, ); } - const actions = [ + const actions: SyncAction[] = [ ...this.determineSyncActions( remoteMetadata.files, this.metadataStore.data.files, ), - ...conflictResolutions, + ...conflictActions, ]; if (actions.length === 0) { @@ -366,7 +361,15 @@ export default class SyncManager { switch (action.type) { case "upload": { const normalizedPath = normalizePath(action.filePath); - const content = await this.vault.adapter.read(normalizedPath); + const resolution = conflictResolutions.find( + (c: ConflictResolution) => c.filename === action.filePath, + ); + // If the file was conflicting we need to read the content from the + // conflict resolution instead of reading it from file since at this point + // we still have not updated the local file. + const content = + resolution?.content || + (await this.vault.adapter.read(normalizedPath)); newTreeFiles[action.filePath] = { path: action.filePath, mode: "100644", @@ -578,15 +581,31 @@ export default class SyncManager { * * @param treeFiles Updated list of files in the remote tree * @param baseTreeSha sha of the tree to use as base for the new tree + * @param conflictResolutions list of conflicts between remote and local files */ async commitSync( treeFiles: { [key: string]: NewTreeRequestItem }, baseTreeSha: string, + conflictResolutions: ConflictResolution[] = [], ) { // Update local sync time - this.metadataStore.data.lastSync = Date.now(); + const syncTime = Date.now(); + this.metadataStore.data.lastSync = syncTime; this.metadataStore.save(); + // We update the last modified timestamp for all files that had resolved conflicts + // to the the same time as the sync time. + // At this time we still have not written the conflict resolution content to file, + // so the last modified timestamp doesn't reflect that. + // To prevent further conflicts in future syncs and to reflect the content change + // on the remote metadata we update the timestamp for the conflicting files here, + // just before pushing to remote. + // We're going to update the local content when the sync is successful. + conflictResolutions.forEach((resolution) => { + this.metadataStore.data.files[resolution.filename].lastModified = + syncTime; + }); + // Update manifest in list of new tree items delete treeFiles[`${this.vault.configDir}/${MANIFEST_FILE_NAME}`].sha; treeFiles[`${this.vault.configDir}/${MANIFEST_FILE_NAME}`].content = @@ -611,6 +630,23 @@ export default class SyncManager { ); await this.client.updateBranchHead(commitSha); + + // Update the local content of all files that had conflicts we resolved + await Promise.all( + conflictResolutions.map(async (resolution) => { + await this.vault.adapter.write(resolution.filename, resolution.content); + // Even though we set the last modified timestamp for all files with conflicts + // just before pushing the changes to remote we do it here again because the + // write right above would overwrite that. + // Since we want to keep the sync timestamp for this file to avoid future conflicts + // we update it again. + this.metadataStore.data.files[resolution.filename].lastModified = + syncTime; + }), + ); + // Now that the sync is done and we updated the content for conflicting files + // we can save the latest metadata to disk. + this.metadataStore.save(); await this.logger.info("Sync done"); } diff --git a/src/views/conflicts-resolution/view.tsx b/src/views/conflicts-resolution/view.tsx index 1c1c427..a4aeb19 100644 --- a/src/views/conflicts-resolution/view.tsx +++ b/src/views/conflicts-resolution/view.tsx @@ -8,78 +8,15 @@ import { ConflictFile, ConflictResolution } from "src/sync-manager"; export const CONFLICTS_RESOLUTION_VIEW_TYPE = "conflicts-resolution-view"; -// Test Case 1: Simple line changes -let oldText1 = `# My Document -This is a test -Some content here -Some line -Another line -Final line`; - -let newText1 = `# My Document -This is a modified test -Some new content here -Some line -Final line`; - -// Test Case 2: Markdown with formatting -let oldText2 = `# Title -## Subtitle -- List item 1 -- List item 2 - -**Bold text** and *italic* text -Regular paragraph`; - -let newText2 = `# Modified Title -## Subtitle -- List item 1 -- List item 2 -- New item - -**Bold text** and *italic* text -Modified paragraph -New paragraph`; - -let oldText3 = `# My Document -This is a modified test -Some new content here - -This is a test -Some content here -Some line -Another line -Final line - - -asdfasdf`; - -let newText3 = `# My Document -This is a modified test -Some new content here - - - - - - -This is a test -Some content here -Some line -Another line -Final line - -asdfasdf`; - export class ConflictsResolutionView extends ItemView { icon: IconName = "merge"; constructor( leaf: WorkspaceLeaf, private plugin: GitHubSyncPlugin, + private conflicts: ConflictFile[], ) { super(leaf); - console.log(`Created ${CONFLICTS_RESOLUTION_VIEW_TYPE}`); } getViewType() { @@ -90,7 +27,7 @@ export class ConflictsResolutionView extends ItemView { return "Conflicts Resolution"; } - onResolve(resolutions: ConflictResolution[]) { + private resolveAllConflicts(resolutions: ConflictResolution[]) { if (this.plugin.conflictsResolver) { this.plugin.conflictsResolver(resolutions); this.plugin.conflictsResolver = null; @@ -98,23 +35,12 @@ export class ConflictsResolutionView extends ItemView { } setConflictFiles(conflicts: ConflictFile[]) { - const mockFiles: ConflictFile[] = [ - { filename: "this", remoteContent: oldText1, localContent: newText1 }, - { filename: "that", remoteContent: oldText2, localContent: newText2 }, - { filename: "those", remoteContent: oldText3, localContent: newText3 }, - ]; - this.render(mockFiles); + this.conflicts = conflicts; + this.render(conflicts); } async onOpen() { - console.log(`Opened ${CONFLICTS_RESOLUTION_VIEW_TYPE}`); - - // const mockFiles: ConflictFile[] = [ - // { filename: "this", remoteContent: oldText1, localContent: newText1 }, - // { filename: "that", remoteContent: oldText2, localContent: newText2 }, - // { filename: "those", remoteContent: oldText3, localContent: newText3 }, - // ]; - // this.render(mockFiles); + this.render(this.conflicts); } private render(conflicts: ConflictFile[]) { @@ -133,24 +59,26 @@ export class ConflictsResolutionView extends ItemView { const onConflictResolved = () => { // Remove the file from the conflicts to resolve - setFiles(files.filter((_, index) => index !== currentFileIndex)); + const remainingFiles = files.filter( + (_, index) => index !== currentFileIndex, + ); + setFiles(remainingFiles); // Keep track of the resolved conflicts - setResolvedConflicts([ + const newResolvedConflicts = [ ...resolvedConflicts, { filename: currentFile!.filename, - // We could get either the local or remote content at this point since - // they're identical content: currentFile!.localContent, }, - ]); + ]; + setResolvedConflicts(newResolvedConflicts); // Select the previous file only if we're not already at the start if (currentFileIndex > 0) { setCurrentFileIndex(currentFileIndex - 1); } - if (files.length === 0) { + if (remainingFiles.length === 0) { // We solved all conflicts, we can resume syncing - this.onResolve(resolvedConflicts); + this.resolveAllConflicts(newResolvedConflicts); } }; @@ -226,6 +154,5 @@ export class ConflictsResolutionView extends ItemView { async onClose() { // Nothing to clean up. - console.log(`Closed ${CONFLICTS_RESOLUTION_VIEW_TYPE}`); } }