Add brief contributing instructions in README.md

.github/workflows/release.yml

@@ -20,14 +20,15 @@ jobs:
 
       - name: Build plugin
         run: |
-          npm install
-          npm run build
+          npm install -g pnpm@latest-10
+          pnpm install
+          pnpm build
 
       - name: Create release
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
-          if [[ "${{ github.ref_name }}" == *-beta ]]; then
+          if [[ "${{ github.ref_name }}" == *-beta* ]]; then
             gh release create "${{ github.ref_name }}" \
               --title="${{ github.ref_name }}" \
               --prerelease \

README.md

@@ -126,6 +126,16 @@ To work correctly this plugin uses a custom metadata file that is updated every
 
 Other plugins don't know about that file, so if you sync with others too you risk losing data.
 
+## Contributing
+
+Contributions are obviously accepted.
+
+Bug fixes are always welcome, new feature must be discussed first. Open a [discussion](https://github.com/silvanocerza/github-gitless-sync/discussions) and let's talk in that case.
+
+Keep PRs as small as possible, I won't review PRs with hundreds of lines if it's mostly code.
+
+Low quality or vibe coded PRs are not welcome. Put some effort in it please.
+
 ## License
 
 The project is licensed under the [AGPLv3](https://www.gnu.org/licenses/agpl-3.0.en.html) license.

manifest.json

@@ -1,7 +1,7 @@
 {
   "id": "github-gitless-sync",
   "name": "GitHub Gitless Sync",
-  "version": "1.0.6",
+  "version": "1.0.7",
   "minAppVersion": "1.7.7",
   "description": "Sync a GitHub repository with vaults on different platforms without requiring git installation",
   "author": "Silvano Cerza",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "github-gitless-sync",
-  "version": "1.0.6",
+  "version": "1.0.7",
   "description": "Sync a GitHub repository with vaults on different platforms without requiring git installation",
   "main": "main.js",
   "scripts": {

src/events-listener.ts

@@ -1,7 +1,7 @@
 import { Vault, TAbstractFile, TFolder } from "obsidian";
 import MetadataStore, { MANIFEST_FILE_NAME } from "./metadata-store";
 import { GitHubSyncSettings } from "./settings/settings";
-import Logger from "./logger";
+import Logger, { LOG_FILE_NAME } from "./logger";
 import GitHubSyncPlugin from "./main";
 
 /**
@@ -145,6 +145,9 @@ export default class EventsListener {
     ) {
       // Obsidian recommends not syncing the workspace files
       return false;
+    } else if (filePath === `${this.vault.configDir}/${LOG_FILE_NAME}`) {
+      // Don't sync the log file, doesn't make sense
+      return false;
     } else if (
       this.settings.syncConfigDir &&
       filePath.startsWith(this.vault.configDir)

src/logger.ts

@@ -1,6 +1,6 @@
 import { Vault, normalizePath } from "obsidian";
 
-const LOG_FILE_NAME = "github-sync.log" as const;
+export const LOG_FILE_NAME = "github-sync.log" as const;
 
 export default class Logger {
   private logFile: string;

src/main.ts

@@ -152,9 +152,19 @@ export default class GitHubSyncPlugin extends Plugin {
       return;
     }
     if (this.settings.firstSync) {
-      await this.syncManager.firstSync();
-      this.settings.firstSync = false;
-      this.saveSettings();
+      const notice = new Notice("Syncing...");
+      try {
+        await this.syncManager.firstSync();
+        this.settings.firstSync = false;
+        this.saveSettings();
+        // Shown only if sync doesn't fail
+        new Notice("Sync successful", 5000);
+      } catch (err) {
+        // Show the error to the user, it's not automatically dismissed to make sure
+        // the user sees it.
+        new Notice(`Error syncing. ${err}`);
+      }
+      notice.hide();
     } else {
       await this.syncManager.sync();
     }

src/sync-manager.ts

@@ -17,7 +17,7 @@ import MetadataStore, {
 } from "./metadata-store";
 import EventsListener from "./events-listener";
 import { GitHubSyncSettings } from "./settings/settings";
-import Logger from "./logger";
+import Logger, { LOG_FILE_NAME } from "./logger";
 import { decodeBase64String, hasTextExtension } from "./utils";
 import GitHubSyncPlugin from "./main";
 import { BlobReader, Entry, Uint8ArrayWriter, ZipReader } from "@zip.js/zip.js";
@@ -95,19 +95,14 @@ export default class SyncManager {
       return;
     }
 
-    const notice = new Notice("Syncing...");
     this.syncing = true;
     try {
       await this.firstSyncImpl();
-      // Shown only if sync doesn't fail
-      new Notice("Sync successful", 5000);
     } catch (err) {
-      // Show the error to the user, it's not automatically dismissed to make sure
-      // the user sees it.
-      new Notice(`Error syncing. ${err}`);
+      this.syncing = false;
+      throw err;
     }
     this.syncing = false;
-    notice.hide();
   }
 
   private async firstSyncImpl() {
@@ -142,9 +137,12 @@ export default class SyncManager {
       // API doesn't let us create a new tree when the repo is empty.
       // So we create a the manifest file as the first commit, since we're going
       // to create that in any case right after this.
+      const buffer = await this.vault.adapter.readBinary(
+        normalizePath(`${this.vault.configDir}/${MANIFEST_FILE_NAME}`),
+      );
       await this.client.createFile({
         path: `${this.vault.configDir}/${MANIFEST_FILE_NAME}`,
-        content: "",
+        content: arrayBufferToBase64(buffer),
         message: "First sync",
         retry: true,
       });
@@ -197,6 +195,10 @@ export default class SyncManager {
     const reader = new ZipReader(new BlobReader(zipBlob));
     const entries = await reader.getEntries();
 
+    await this.logger.info("Extracting files from ZIP", {
+      length: entries.length,
+    });
+
     await Promise.all(
       entries.map(async (entry: Entry) => {
         // All repo ZIPs contain a root directory that contains all the content
@@ -206,16 +208,43 @@ export default class SyncManager {
         const targetPath =
           pathParts.length > 1 ? pathParts.slice(1).join("/") : entry.filename;
 
+        if (targetPath === "") {
+          // Must be the root folder, skip it.
+          // This is really important as that would lead us to try and
+          // create the folder "/" and crash Obsidian
+          return;
+        }
+
         if (
           this.settings.syncConfigDir &&
           targetPath.startsWith(this.vault.configDir) &&
           targetPath !== `${this.vault.configDir}/${MANIFEST_FILE_NAME}`
         ) {
+          await this.logger.info("Skipped config", { targetPath });
           return;
         }
 
         if (entry.directory) {
-          await this.vault.adapter.mkdir(normalizePath(targetPath));
+          const normalizedPath = normalizePath(targetPath);
+          await this.vault.adapter.mkdir(normalizedPath);
+          await this.logger.info("Created directory", {
+            normalizedPath,
+          });
+          return;
+        }
+
+        if (targetPath === `${this.vault.configDir}/${LOG_FILE_NAME}`) {
+          // We don't want to download the log file if the user synced it in the past.
+          // This is necessary because in the past we forgot to ignore the log file
+          // from syncing if the user enabled configs sync.
+          // To avoid downloading it we ignore it if still present in the remote repo.
+          return;
+        }
+
+        if (targetPath.split("/").last()?.startsWith(".")) {
+          // We must skip hidden files as that creates issues with syncing.
+          // This is fine as users can't edit hidden files in Obsidian anyway.
+          await this.logger.info("Skipping hidden file", targetPath);
           return;
         }
 
@@ -224,12 +253,31 @@ export default class SyncManager {
         const data = await writer.getData();
         const dir = targetPath.split("/").splice(0, -1).join("/");
         if (dir !== "") {
-          await this.vault.adapter.mkdir(normalizePath(dir));
+          const normalizedDir = normalizePath(dir);
+          await this.vault.adapter.mkdir(normalizedDir);
+          await this.logger.info("Created directory", {
+            normalizedDir,
+          });
         }
-        await this.vault.adapter.writeBinary(normalizePath(targetPath), data);
+
+        const normalizedPath = normalizePath(targetPath);
+        await this.vault.adapter.writeBinary(normalizedPath, data);
+        await this.logger.info("Written file", {
+          normalizedPath,
+        });
+        this.metadataStore.data.files[normalizedPath] = {
+          path: normalizedPath,
+          sha: files[normalizedPath].sha,
+          dirty: false,
+          justDownloaded: true,
+          lastModified: Date.now(),
+        };
+        await this.metadataStore.save();
       }),
     );
 
+    await this.logger.info("Extracted zip");
+
     const newTreeFiles = Object.keys(files)
       .map((filePath: string) => ({
         path: files[filePath].path,
@@ -246,18 +294,33 @@ export default class SyncManager {
       );
     // Add files that are in the manifest but not in the tree.
     await Promise.all(
-      Object.keys(this.metadataStore.data.files).map(
-        async (filePath: string) => {
+      Object.keys(this.metadataStore.data.files)
+        .filter((filePath: string) => {
+          return !Object.keys(files).contains(filePath);
+        })
+        .map(async (filePath: string) => {
           const normalizedPath = normalizePath(filePath);
-          const content = await this.vault.adapter.read(normalizedPath);
+          // We need to check whether the file is a text file or not before
+          // reading it here because trying to read a binary file as text fails
+          // on iOS, and probably on other mobile devices too, so we read the file
+          // content only if we're sure it contains text only.
+          //
+          // It's fine not reading the binary file in here and just setting some bogus
+          // content because when committing the sync we're going to read the binary
+          // file and upload its blob if it needs to be synced. The important thing is
+          // that some content is set so we know the file changed locally and needs to be
+          // uploaded.
+          let content = "binaryfile";
+          if (hasTextExtension(normalizedPath)) {
+            content = await this.vault.adapter.read(normalizedPath);
+          }
           newTreeFiles[filePath] = {
             path: filePath,
             mode: "100644",
             type: "blob",
-            content: content,
+            content,
           };
-        },
-      ),
+        }),
     );
     await this.commitSync(newTreeFiles, treeSha);
   }
@@ -290,18 +353,36 @@ export default class SyncManager {
         {},
       );
     await Promise.all(
-      Object.keys(this.metadataStore.data.files).map(
-        async (filePath: string) => {
+      Object.keys(this.metadataStore.data.files)
+        .filter((filePath: string) => {
+          // We should not try to sync deleted files, this can happen when
+          // the user renames or deletes files after enabling the plugin but
+          // before syncing for the first time
+          return !this.metadataStore.data.files[filePath].deleted;
+        })
+        .map(async (filePath: string) => {
           const normalizedPath = normalizePath(filePath);
-          const content = await this.vault.adapter.read(normalizedPath);
+          // We need to check whether the file is a text file or not before
+          // reading it here because trying to read a binary file as text fails
+          // on iOS, and probably on other mobile devices too, so we read the file
+          // content only if we're sure it contains text only.
+          //
+          // It's fine not reading the binary file in here and just setting some bogus
+          // content because when committing the sync we're going to read the binary
+          // file and upload its blob if it needs to be synced. The important thing is
+          // that some content is set so we know the file changed locally and needs to be
+          // uploaded.
+          let content = "binaryfile";
+          if (hasTextExtension(normalizedPath)) {
+            content = await this.vault.adapter.read(normalizedPath);
+          }
           newTreeFiles[filePath] = {
             path: filePath,
             mode: "100644",
             type: "blob",
-            content: content,
+            content,
           };
-        },
-      ),
+        }),
     );
     await this.commitSync(newTreeFiles, treeSha);
   }
@@ -344,6 +425,16 @@ export default class SyncManager {
       throw new Error("Remote manifest is missing");
     }
 
+    if (
+      Object.keys(files).contains(`${this.vault.configDir}/${LOG_FILE_NAME}`)
+    ) {
+      // We don't want to download the log file if the user synced it in the past.
+      // This is necessary because in the past we forgot to ignore the log file
+      // from syncing if the user enabled configs sync.
+      // To avoid downloading it we delete it if still around.
+      delete files[`${this.vault.configDir}/${LOG_FILE_NAME}`];
+    }
+
     const blob = await this.client.getBlob({ sha: manifest.sha });
     const remoteMetadata: Metadata = JSON.parse(
       decodeBase64String(blob.content),
@@ -609,16 +700,19 @@ export default class SyncManager {
               type: "delete_local",
               filePath: filePath,
             });
+            return;
           } else if (
             localFile.lastModified > (remoteFile.deletedAt as number)
           ) {
             actions.push({ type: "upload", filePath: filePath });
+            return;
           }
         }
 
         if (!remoteFile.deleted && localFile.deleted) {
           if (remoteFile.lastModified > (localFile.deletedAt as number)) {
             actions.push({ type: "download", filePath: filePath });
+            return;
           } else if (
             (localFile.deletedAt as number) > remoteFile.lastModified
           ) {
@@ -626,6 +720,7 @@ export default class SyncManager {
               type: "delete_remote",
               filePath: filePath,
             });
+            return;
           }
         }
 
@@ -633,8 +728,10 @@ export default class SyncManager {
         // Conflicts are already filtered out so we can make this decision easily
         if (localSHA !== localFile.sha) {
           actions.push({ type: "upload", filePath: filePath });
+          return;
         } else {
           actions.push({ type: "download", filePath: filePath });
+          return;
         }
       }),
     );
@@ -693,9 +790,13 @@ export default class SyncManager {
    * This is the same identical algoritm used by git to calculate
    * a blob's SHA.
    * @param filePath normalized path to file
-   * @returns String containing the file SHA1
+   * @returns String containing the file SHA1 or null in case the file doesn't exist
    */
-  async calculateSHA(filePath: string): Promise<string> {
+  async calculateSHA(filePath: string): Promise<string | null> {
+    if (!(await this.vault.adapter.exists(filePath))) {
+      // The file doesn't exist, can't calculate any SHA
+      return null;
+    }
     const contentBuffer = await this.vault.adapter.readBinary(filePath);
     const contentBytes = new Uint8Array(contentBuffer);
     const header = new TextEncoder().encode(`blob ${contentBytes.length}\0`);
@@ -770,6 +871,7 @@ export default class SyncManager {
             retry: true,
             maxRetries: 3,
           });
+          await this.logger.info("Created blob", filePath);
           treeFiles[filePath].sha = sha;
           // Can't have both sha and content set, so we delete it
           delete treeFiles[filePath].content;

src/views/conflicts-resolution/split-view/actions-gutter.tsx

@@ -175,7 +175,6 @@ const ActionsGutter: React.FC<ActionsGutterProps> = ({
       style={{
         width: "100%",
         height: "100%",
-        overflow: "hidden",
         position: "relative",
         backgroundColor: "var(--background-secondary)",
       }}

src/views/conflicts-resolution/split-view/diff-view.tsx

@@ -36,13 +36,11 @@ const DiffView: React.FC<DiffViewProps> = ({
   return (
     <div
       style={{
-        width: "100%",
         height: "100%",
         display: "flex",
-        overflow: "hidden",
       }}
     >
-      <div style={{ flex: 1, overflow: "hidden" }}>
+      <div style={{ flex: 1 }}>
         <EditorPane
           content={remoteText}
           highlightPluginSpec={{
@@ -142,7 +140,7 @@ const DiffView: React.FC<DiffViewProps> = ({
           }}
         />
       </div>
-      <div style={{ flex: 1, overflow: "hidden" }}>
+      <div style={{ flex: 1 }}>
         <EditorPane
           content={localText}
           highlightPluginSpec={{

src/views/conflicts-resolution/split-view/split-view.tsx

@@ -46,9 +46,9 @@ const SplitView = ({
     <React.StrictMode>
       <div
         style={{
-          height: "100%",
           display: "flex",
           flexDirection: "column",
+          height: "100%",
           justifyContent: "center",
         }}
       >
@@ -88,21 +88,28 @@ const SplitView = ({
               currentFile={currentFile?.filePath || ""}
               setCurrentFileIndex={setCurrentFileIndex}
             />
-            <DiffView
-              remoteText={currentFile?.remoteContent || ""}
-              localText={currentFile?.localContent || ""}
-              onRemoteTextChange={(content: string) => {
-                const tempFiles = [...files];
-                tempFiles[currentFileIndex].remoteContent = content;
-                setFiles(tempFiles);
+            <div
+              style={{
+                overflow: "auto",
+                flex: 1,
               }}
-              onLocalTextChange={(content: string) => {
-                const tempFiles = [...files];
-                tempFiles[currentFileIndex].localContent = content;
-                setFiles(tempFiles);
-              }}
-              onConflictResolved={onConflictResolved}
-            />
+            >
+              <DiffView
+                remoteText={currentFile?.remoteContent || ""}
+                localText={currentFile?.localContent || ""}
+                onRemoteTextChange={(content: string) => {
+                  const tempFiles = [...files];
+                  tempFiles[currentFileIndex].remoteContent = content;
+                  setFiles(tempFiles);
+                }}
+                onLocalTextChange={(content: string) => {
+                  const tempFiles = [...files];
+                  tempFiles[currentFileIndex].localContent = content;
+                  setFiles(tempFiles);
+                }}
+                onConflictResolved={onConflictResolved}
+              />
+            </div>
           </>
         )}
       </div>

src/views/conflicts-resolution/unified-view/diff-view.tsx

@@ -183,7 +183,15 @@ const DiffView: React.FC<DiffViewProps> = ({
   }, [initialRemoteText, initialLocalText]);
 
   return (
-    <div style={{ width: "100%", height: "100%", overflow: "hidden" }}>
+    <div
+      style={{
+        width: "100%",
+        height: "100%",
+        display: "flex",
+        flexDirection: "column",
+        overflow: "hidden",
+      }}
+    >
       <CodeMirror
         value={doc}
         height="100%"

src/views/conflicts-resolution/unified-view/unified-view.tsx

@@ -42,6 +42,8 @@ const UnifiedView = ({
           paddingTop: "var(--size-4-4)",
           paddingBottom: "var(--size-4-4)",
           borderBottom: "1px solid var(--background-modifier-border)",
+          display: "flex",
+          flexDirection: "column",
         }}
       >
         <div
@@ -67,7 +69,14 @@ const UnifiedView = ({
 
   return (
     <React.StrictMode>
-      <div>
+      <div
+        style={{
+          height: "100%",
+          display: "flex",
+          flexDirection: "column",
+          overflow: "auto",
+        }}
+      >
         {files.length === 0 ? (
           <div
             style={{

styles.css

@@ -4,4 +4,6 @@
 
 .padless-conflicts-view-container {
     padding: 0;
+    height: 100%;
+    width: 100%;
 }

versions.json

@@ -12,5 +12,6 @@
   "1.0.3": "1.7.7",
   "1.0.4": "1.7.7",
   "1.0.5": "1.7.7",
-  "1.0.6": "1.7.7"
+  "1.0.6": "1.7.7",
+  "1.0.7": "1.7.7"
 }