#754 and #865

* Fix #754

The pragma util keeps a copy of the settings that are upload to the
gist. The file is located at <context.globalStoragePath>/settings.sync.

If the content to be upload doesn't deffer from the local content after
taking out the @sync-ignore, the file wont be uploaded.

* Fix #754: Sync object

Checks the global storage path at constructor.
Checks if the content of the setting file should be uploaded.

* Update tests

* Fix #865

* Fix broken tests

* Fixing Octokit wrong type for GistsResponse

'file' field is wrong with static data.

* Defining PragmaUtil behavior

Pragma util should only parse string content, checking each line for pragma statements.

* Updating test for Pragma definition

* Sync: don't upload is not necessary

As the extension gets the entire gists before commiting an upload, we can check each file of the gists against our local files in order to determine if the content has changed.

There is no need to keel a local copy.

src/pragmaUtil.ts

@@ -122,7 +122,9 @@ export default class PragmaUtil {
    * @returns {string}
    * @memberof PragmaUtil
    */
-  public static processBeforeUpload(fileContent: string): string {
+  public static async processBeforeUpload(
+    fileContent: string
+  ): Promise<string> {
     const lines = fileContent.split("\n");
     let osMatch: RegExpMatchArray;
     let osFromPragma: string;
@@ -195,7 +197,6 @@ export default class PragmaUtil {
         parsedLines.push(currentLine);
       }
     }
-
     return parsedLines.join("\n");
   }
 
@@ -233,14 +234,15 @@ export default class PragmaUtil {
 
   private static toggleComments(line: string, shouldComment: boolean) {
     const isCommented = line.trim().startsWith("//");
-
     if (shouldComment) {
       // Replace with RegEx to help match indent size
       return !isCommented ? line.replace(/^(\s*)/, "$1// ") : line;
     } else {
       // Only remove if line is commented
       return isCommented ? line.replace(/\/\/\s*/, "") : line;
     }
+
+    return line;
   }
 
   // checks and advance index

src/service/github.service.ts

@@ -11,6 +11,10 @@ interface IEnv {
   HTTP_PROXY: string;
 }
 
+interface IFixGistResponse extends Omit<GitHubApi.GistsGetResponse, "files"> {
+  files: any | GitHubApi.GistsGetResponseFiles;
+}
+
 export class GitHubService {
   public userName: string = null;
   public name: string = null;
@@ -120,7 +124,10 @@ export class GitHubService {
     }
   }
 
-  public async ReadGist(GIST: string): Promise<any> {
+  // This should return GitHubApi.Response<GitHubApi.GistsGetResponse> but Types are wrong
+  public async ReadGist(
+    GIST: string
+  ): Promise<GitHubApi.Response<IFixGistResponse>> {
     return await this.github.gists.get({ gist_id: GIST });
   }
 

src/sync.ts

@@ -82,7 +82,11 @@ export class Sync {
         localConfig.customConfig.token,
         localConfig.customConfig.githubEnterpriseUrl
       );
-      await startGitProcess(localConfig.extConfig, localConfig.customConfig);
+      await startGitProcess.call(
+        this,
+        localConfig.extConfig,
+        localConfig.customConfig
+      );
     } catch (error) {
       Commons.LogException(error, state.commons.ERROR_MESSAGE, true);
       return;
@@ -200,7 +204,6 @@ export class Sync {
         Commons.LogException(null, state.commons.ERROR_MESSAGE, true);
         return;
       }
-
       for (const snippetFile of contentFiles) {
         if (snippetFile.fileName !== state.environment.FILE_KEYBINDING_MAC) {
           if (snippetFile.content !== "") {
@@ -213,23 +216,23 @@ export class Sync {
                   ? state.environment.FILE_KEYBINDING_MAC
                   : state.environment.FILE_KEYBINDING_DEFAULT;
             }
-            allSettingFiles.push(snippetFile);
-          }
-        }
-
-        if (
-          snippetFile.fileName === state.environment.FILE_SETTING_NAME ||
-          snippetFile.fileName === state.environment.FILE_KEYBINDING_MAC ||
-          snippetFile.fileName === state.environment.FILE_KEYBINDING_DEFAULT
-        ) {
-          try {
-            snippetFile.content = PragmaUtil.processBeforeUpload(
-              snippetFile.content
-            );
-          } catch (e) {
-            Commons.LogException(null, e.message, true);
-            console.error(e);
-            return;
+            if (
+              snippetFile.fileName === state.environment.FILE_SETTING_NAME ||
+              snippetFile.fileName === state.environment.FILE_KEYBINDING_MAC ||
+              snippetFile.fileName === state.environment.FILE_KEYBINDING_DEFAULT
+            ) {
+              try {
+                const parsedContent = await PragmaUtil.processBeforeUpload(
+                  snippetFile.content
+                );
+                snippetFile.content = parsedContent;
+                allSettingFiles.push(snippetFile);
+              } catch (e) {
+                Commons.LogException(null, e.message, true);
+                console.error(e);
+                return;
+              }
+            }
           }
         }
       }
@@ -296,14 +299,37 @@ export class Sync {
           }
         }
 
-        if (gistObj.public === true) {
+        if (gistObj.data.public === true) {
           localConfig.publicGist = true;
         }
 
         vscode.window.setStatusBarMessage(
           localize("cmd.updateSettings.info.uploadingFile"),
           3000
         );
+
+        // We should not upload unless one or more files have changed
+        let shouldUploadGist = false;
+        for (const fileToUpload of allSettingFiles) {
+          // This file contains only file time stamps
+          if (fileToUpload.fileName === "cloudSettings") {
+            continue;
+          }
+          if (
+            gistObj.data.files[fileToUpload.fileName].content !==
+            fileToUpload.content
+          ) {
+            console.info(`Sync: file ${fileToUpload.fileName} has changed`);
+            shouldUploadGist = true;
+            break;
+          }
+        }
+
+        if (!shouldUploadGist) {
+          console.info("Sync: nothing to update");
+          return;
+        }
+
         gistObj = github.UpdateGIST(gistObj, allSettingFiles);
         completed = await github.SaveGIST(gistObj.data);
         if (!completed) {

test/pragmaUtil/index.ts

@@ -13,23 +13,25 @@ describe("Process before upload", function() {
     );
   });
 
-  it("should trim os, host and env", () => {
-    expect(PragmaUtil.processBeforeUpload(testSettings)).to.match(
-      /@sync os=linux host=trim env=TEST_ENV/
-    );
+  it("should trim os, host and env", async () => {
+    const result = await PragmaUtil.processBeforeUpload(testSettings);
+    await expect(result).to.match(/@sync os=linux host=trim env=TEST_ENV/);
   });
 
-  it("should uncomment all lines", () => {
+  it("should uncomment all lines", async () => {
     const commentedSettings = `
       // @sync os=linux
       // "window": 1,
       // @sync os=mac
-      // "mac": 1
+      // "server": "http://exmaple.com
     `;
 
-    expect(PragmaUtil.processBeforeUpload(commentedSettings))
+    const result = await PragmaUtil.processBeforeUpload(
+      commentedSettings
+    );
+    await expect(result)
       .to.match(/\s+"window"/)
-      .and.to.match(/\s+"mac"/);
+      .and.to.match(/\s+"server"/);
   });
 
   it("should uncomment lines before write file for os=linux", () => {
@@ -47,11 +49,11 @@ describe("Process before upload", function() {
     );
     expect(processed)
       .to.match(/\s+"linux"/)
-      .and.to.match(/.+\/\/"mac"/);
+      .and.to.match(/\s+\/\/\s+"mac"/);
   });
 
-  it("should not comment os=linux settings lines", () => {
-    let processed = PragmaUtil.processBeforeUpload(testSettings);
+  it("should not comment os=linux settings lines", async () => {
+    let processed = await PragmaUtil.processBeforeUpload(testSettings);
     processed = PragmaUtil.processBeforeWrite(
       processed,
       processed,
@@ -61,11 +63,13 @@ describe("Process before upload", function() {
     expect(processed).to.match(/\s+"not_commented"/);
   });
 
-  it("should leave only settings that matches with os=mac host=mac2 env=TEST_ENV", () => {
-    const processed = PragmaUtil.processBeforeUpload(testSettings);
+  it("should leave only settings that matches with os=mac host=mac2 env=TEST_ENV", async () => {
+    const processed = await PragmaUtil.processBeforeUpload(
+      testSettings
+    );
     // tslint:disable-next-line:no-string-literal
     process.env["TEST_ENV"] = "1";
-    expect(
+    await expect(
       PragmaUtil.processBeforeWrite(processed, processed, OsType.Mac, "mac2")
     )
       .to.match(/\n\s+"mac2"/)
@@ -98,5 +102,5 @@ describe("Process before upload", function() {
       .and.to.match(/\/{2}\s+"setting"/)
       .and.to.match(/\/{2}\s+},/)
       .and.to.match(/\s+"mac"/);
-  })
+  });
 });

test/pragmaUtil/testSettings.txt

@@ -18,6 +18,9 @@
     // @sync host=mac2 os=mac env=TEST_ENV
     //"mac2": 3,
 
+    // @sync host=mac2
+    //"server": "http://example.com",
+
     // @sync os=mac
     "mactest": "",