Rework how conflicts are passed from SyncManager to ConflictsResolutionView

src/main.ts

@@ -1,7 +1,7 @@
 import { EventRef, Plugin, Platform, WorkspaceLeaf } from "obsidian";
 import { GitHubSyncSettings, DEFAULT_SETTINGS } from "./settings/settings";
 import GitHubSyncSettingsTab from "./settings/tab";
-import SyncManager from "./sync-manager";
+import SyncManager, { ConflictFile, ConflictResolution } from "./sync-manager";
 import { FileMetadata } from "./metadata-store";
 import { OnboardingDialog } from "./views/onboarding/view";
 import Logger from "./logger";
@@ -22,6 +22,13 @@ export default class GitHubSyncPlugin extends Plugin {
   vaultCreateListener: EventRef | null = null;
   vaultModifyListener: EventRef | null = null;
 
+  // Called in ConflictResolutionView when the user solves all the conflicts.
+  // This is initialized every time we open the view to set new conflicts so
+  // we can notify the SyncManager that everything has been resolved and the sync
+  // process can continue on.
+  conflictsResolver: ((resolutions: ConflictResolution[]) => void) | null =
+    null;
+
   async onUserEnable() {
     if (Platform.isMobile) {
       // TODO: Implement onboarding for mobile
@@ -34,6 +41,16 @@ export default class GitHubSyncPlugin extends Plugin {
     }
   }
 
+  getConflictsView(): ConflictsResolutionView | null {
+    const leaves = this.app.workspace.getLeavesOfType(
+      CONFLICTS_RESOLUTION_VIEW_TYPE,
+    );
+    if (leaves.length === 0) {
+      return null;
+    }
+    return leaves[0].view as ConflictsResolutionView;
+  }
+
   async activateView() {
     const { workspace } = this.app;
     let leaf: WorkspaceLeaf | null = null;
@@ -41,7 +58,7 @@ export default class GitHubSyncPlugin extends Plugin {
     if (leaves.length > 0) {
       leaf = leaves[0];
     } else {
-      leaf = workspace.getRightLeaf(false);
+      leaf = workspace.getRightLeaf(false)!;
       await leaf.setViewState({
         type: CONFLICTS_RESOLUTION_VIEW_TYPE,
         active: true,
@@ -57,11 +74,10 @@ export default class GitHubSyncPlugin extends Plugin {
       CONFLICTS_RESOLUTION_VIEW_TYPE,
       (leaf) => new ConflictsResolutionView(leaf, this),
     );
-    this.addRibbonIcon(
-      "dice",
-      "Activate view",
-      async () => await this.activateView(),
-    );
+    this.addRibbonIcon("dice", "Activate view", async () => {
+      await this.activateView();
+      this.getConflictsView()?.setConflictFiles([]);
+    });
     this.logger = new Logger(this.app.vault, this.settings.enableLogging);
 
     this.addSettingTab(new GitHubSyncSettingsTab(this.app, this));
@@ -182,24 +198,12 @@ export default class GitHubSyncPlugin extends Plugin {
     this.uploadModifiedFilesRibbonIcon = null;
   }
 
-  async onConflicts(
-    conflicts: { remoteFile: FileMetadata; localFile: FileMetadata }[],
-  ): Promise<boolean[]> {
-    return await Promise.all(
-      conflicts.map(
-        async ({
-          remoteFile,
-          localFile,
-        }: {
-          remoteFile: FileMetadata;
-          localFile: FileMetadata;
-        }) => {
-          // TODO: Add a proper conflict resolution view
-          // This way remote files are always preferred
-          return true;
-        },
-      ),
-    );
+  async onConflicts(conflicts: ConflictFile[]): Promise<ConflictResolution[]> {
+    return await new Promise(async (resolve) => {
+      this.conflictsResolver = resolve;
+      await this.activateView();
+      this.getConflictsView()?.setConflictFiles(conflicts);
+    });
   }
 
   async loadSettings() {

src/sync-manager.ts

@@ -18,9 +18,20 @@ interface SyncAction {
   filePath: string;
 }
 
+export interface ConflictFile {
+  filename: string;
+  remoteContent: string;
+  localContent: string;
+}
+
+export interface ConflictResolution {
+  filename: string;
+  content: string;
+}
+
 type OnConflictsCallback = (
-  conflicts: { remoteFile: FileMetadata; localFile: FileMetadata }[],
-) => Promise<boolean[]>;
+  conflicts: ConflictFile[],
+) => Promise<ConflictResolution[]>;
 
 export default class SyncManager {
   private metadataStore: MetadataStore;
@@ -290,10 +301,7 @@ export default class SyncManager {
     const blob = await this.client.getBlob(manifest.sha);
     const remoteMetadata: Metadata = JSON.parse(atob(blob.content));
 
-    const conflicts = this.findConflicts(
-      remoteMetadata.files,
-      this.metadataStore.data.files,
-    );
+    const conflicts = await this.findConflicts(remoteMetadata.files);
     let conflictResolutions: SyncAction[] = [];
     if (conflicts.length > 0) {
       // TODO: Show conflicts to the user with a callback
@@ -401,47 +409,51 @@ export default class SyncManager {
 
   /**
    * Finds conflicts between local and remote files.
-   * @param remoteFiles All files in the remote repo
-   * @param localFiles All files in the local vault
-   * @returns List of objects with both remote and local conflicting files metadata
+   * @param filesMetadata Remote files metadata
+   * @returns List of object containing filename, remote and local content of conflicting files
    */
-  findConflicts(
-    remoteFiles: { [key: string]: FileMetadata },
-    localFiles: { [key: string]: FileMetadata },
-  ): { remoteFile: FileMetadata; localFile: FileMetadata }[] {
-    const commonFiles = Object.keys(remoteFiles).filter(
-      (key) => key in localFiles,
+  async findConflicts(filesMetadata: {
+    [key: string]: FileMetadata;
+  }): Promise<ConflictFile[]> {
+    const commonFiles = Object.keys(filesMetadata).filter(
+      (key) => key in this.metadataStore.data.files,
     );
+    if (commonFiles.length === 0) {
+      return [];
+    }
 
-    return commonFiles
-      .map((filePath: string) => {
-        const remoteFile = remoteFiles[filePath];
-        const localFile = localFiles[filePath];
-
-        // We compare the SHA cause it remote files changes the SHA changes
-        // but not the same happens when the file is modified locally.
-        // So if sha are different and the local file is newer we can't
-        // know for sure which version should be kept.
-        if (
-          remoteFile.sha !== localFile.sha &&
-          remoteFile.lastModified < localFile.lastModified
-        ) {
-          // File is modified on both sides, the user must solve the conflict
+    return await Promise.all(
+      commonFiles
+        .filter((filePath: string) => {
+          // We compare the SHA cause if the remote file changes the SHA changes,
+          // but not the same happens when the file is modified locally.
+          // So if the SHAs are different and the local file is newer we can't
+          // know for sure which version should be kept and that's a conflict.
+          const remoteFile = filesMetadata[filePath];
+          const localFile = this.metadataStore.data.files[filePath];
+          return (
+            remoteFile.sha !== localFile.sha &&
+            remoteFile.lastModified < localFile.lastModified
+          );
+        })
+        .map(async (filePath: string) => {
+          // Load contents in parallel
+          const [remoteContent, localContent] = await Promise.all([
+            await (async () => {
+              const res = await this.client.getBlob(
+                filesMetadata[filePath].sha!,
+              );
+              return atob(res.content);
+            })(),
+            await this.vault.adapter.read(normalizePath(filePath)),
+          ]);
           return {
-            remoteFile: remoteFile,
-            localFile: localFile,
+            filename: filePath,
+            remoteContent: remoteContent,
+            localContent: localContent,
           };
-        }
-        return null;
-      })
-      .filter(
-        (
-          conflict: {
-            remoteFile: FileMetadata;
-            localFile: FileMetadata;
-          } | null,
-        ) => conflict !== null,
-      );
+        }),
+    );
   }
 
   /**

src/views/conflicts-resolution/view.tsx

@@ -4,6 +4,7 @@ import DiffView from "./component";
 import GitHubSyncPlugin from "src/main";
 import * as React from "react";
 import FilesTabBar from "./files-tab-bar";
+import { ConflictFile, ConflictResolution } from "src/sync-manager";
 
 export const CONFLICTS_RESOLUTION_VIEW_TYPE = "conflicts-resolution-view";
 
@@ -70,12 +71,6 @@ Final line
 
 asdfasdf`;
 
-interface ConflictFile {
-  filename: string;
-  remoteContent: string;
-  localContent: string;
-}
-
 export class ConflictsResolutionView extends ItemView {
   icon: IconName = "merge";
 
@@ -84,6 +79,7 @@ export class ConflictsResolutionView extends ItemView {
     private plugin: GitHubSyncPlugin,
   ) {
     super(leaf);
+    console.log(`Created ${CONFLICTS_RESOLUTION_VIEW_TYPE}`);
   }
 
   getViewType() {
@@ -94,16 +90,38 @@ export class ConflictsResolutionView extends ItemView {
     return "Conflicts Resolution";
   }
 
-  async onOpen() {
-    const container = this.containerEl.children[1];
-    container.empty();
-    // We don't want any padding, the DiffView component will handle that
-    (container as HTMLElement).style.padding = "0";
-    const files: ConflictFile[] = [
+  onResolve(resolutions: ConflictResolution[]) {
+    if (this.plugin.conflictsResolver) {
+      this.plugin.conflictsResolver(resolutions);
+      this.plugin.conflictsResolver = null;
+    }
+  }
+
+  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);
+  }
+
+  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);
+  }
+
+  private render(conflicts: ConflictFile[]) {
+    const container = this.containerEl.children[1];
+    container.empty();
+    // We don't want any padding, the DiffView component will handle that
+    (container as HTMLElement).style.padding = "0";
     const root: Root = createRoot(container);
     const App = ({ initialFiles }: { initialFiles: ConflictFile[] }) => {
       const [files, setFiles] = React.useState(initialFiles);
@@ -130,23 +148,22 @@ export class ConflictsResolutionView extends ItemView {
                 const tempFiles = [...files];
                 tempFiles[currentFileIndex].remoteContent = content;
                 setFiles(tempFiles);
-                // currentFile.remoteContent = content;
               }}
               onNewTextChange={(content: string) => {
                 const tempFiles = [...files];
                 tempFiles[currentFileIndex].localContent = content;
                 setFiles(tempFiles);
-                // currentFile.localContent = content;
               }}
             />
           </div>
         </React.StrictMode>
       );
     };
-    root.render(<App initialFiles={files} />);
+    root.render(<App initialFiles={conflicts} />);
   }
 
   async onClose() {
     // Nothing to clean up.
+    console.log(`Closed ${CONFLICTS_RESOLUTION_VIEW_TYPE}`);
   }
 }