Merge pull request #676 from ianhi/update-open-files

Update open files when Git commands modify them

Author: fcollonval

jupyterlab_git/git.py

@@ -184,7 +184,7 @@ async def config(self, top_repo_path, **kwargs):
 
         return response
 
-    async def changed_files(self, base=None, remote=None, single_commit=None):
+    async def changed_files(self, current_path, base=None, remote=None, single_commit=None):
         """Gets the list of changed files between two Git refs, or the files changed in a single commit
 
         There are two reserved "refs" for the base
@@ -204,7 +204,7 @@ async def changed_files(self, base=None, remote=None, single_commit=None):
             }
         """
         if single_commit:
-            cmd = ["git", "diff", "{}^!".format(single_commit), "--name-only", "-z"]
+            cmd = ["git", "diff", single_commit, "--name-only", "-z"]
         elif base and remote:
             if base == "WORKING":
                 cmd = ["git", "diff", remote, "--name-only", "-z"]
@@ -219,7 +219,7 @@ async def changed_files(self, base=None, remote=None, single_commit=None):
 
         response = {}
         try:
-            code, output, error = await execute(cmd, cwd=self.root_dir)
+            code, output, error = await execute(cmd, cwd=os.path.join(self.root_dir, current_path))
         except subprocess.CalledProcessError as e:
             response["code"] = e.returncode
             response["message"] = e.output.decode("utf-8")

jupyterlab_git/tests/test_diff.py

@@ -14,7 +14,7 @@
 async def test_changed_files_invalid_input():
     with pytest.raises(tornado.web.HTTPError):
         await Git(FakeContentManager("/bin")).changed_files(
-            base="64950a634cd11d1a01ddfedaeffed67b531cb11e"
+            current_path="test-path", base="64950a634cd11d1a01ddfedaeffed67b531cb11e"
         )
 
 
@@ -29,7 +29,7 @@ async def test_changed_files_single_commit():
 
         # When
         actual_response = await Git(FakeContentManager("/bin")).changed_files(
-            single_commit="64950a634cd11d1a01ddfedaeffed67b531cb11e"
+            current_path="test-path", single_commit="64950a634cd11d1a01ddfedaeffed67b531cb11e^!"
         )
 
         # Then
@@ -41,7 +41,7 @@ async def test_changed_files_single_commit():
                 "--name-only",
                 "-z",
             ],
-            cwd="/bin",
+            cwd="/bin/test-path",
         )
         assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response
 
@@ -56,12 +56,12 @@ async def test_changed_files_working_tree():
 
         # When
         actual_response = await Git(FakeContentManager("/bin")).changed_files(
-            base="WORKING", remote="HEAD"
+            current_path="test-path", base="WORKING", remote="HEAD"
         )
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "HEAD", "--name-only", "-z"], cwd="/bin"
+            ["git", "diff", "HEAD", "--name-only", "-z"], cwd="/bin/test-path"
         )
         assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response
 
@@ -76,12 +76,12 @@ async def test_changed_files_index():
 
         # When
         actual_response = await Git(FakeContentManager("/bin")).changed_files(
-            base="INDEX", remote="HEAD"
+            current_path="test-path", base="INDEX", remote="HEAD"
         )
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "--staged", "HEAD", "--name-only", "-z"], cwd="/bin"
+            ["git", "diff", "--staged", "HEAD", "--name-only", "-z"], cwd="/bin/test-path"
         )
         assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response
 
@@ -96,12 +96,12 @@ async def test_changed_files_two_commits():
 
         # When
         actual_response = await Git(FakeContentManager("/bin")).changed_files(
-            base="HEAD", remote="origin/HEAD"
+            current_path = "test-path", base="HEAD", remote="origin/HEAD"
         )
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin"
+            ["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin/test-path"
         )
         assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response
 
@@ -114,12 +114,12 @@ async def test_changed_files_git_diff_error():
 
         # When
         actual_response = await Git(FakeContentManager("/bin")).changed_files(
-            base="HEAD", remote="origin/HEAD"
+            current_path="test-path", base="HEAD", remote="origin/HEAD"
         )
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin"
+            ["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin/test-path"
         )
         assert {"code": 128, "message": "error message"} == actual_response
 

src/index.ts

@@ -5,6 +5,7 @@ import {
 } from '@jupyterlab/application';
 import { Dialog, showErrorMessage } from '@jupyterlab/apputils';
 import { IChangedArgs } from '@jupyterlab/coreutils';
+import { IDocumentManager } from '@jupyterlab/docmanager';
 import { FileBrowserModel, IFileBrowserFactory } from '@jupyterlab/filebrowser';
 import { IMainMenu } from '@jupyterlab/mainmenu';
 import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
@@ -32,6 +33,7 @@ const plugin: JupyterFrontEndPlugin<IGitExtension> = {
     IFileBrowserFactory,
     IRenderMimeRegistry,
     ISettingRegistry,
+    IDocumentManager,
     IStatusBar
   ],
   provides: IGitExtension,
@@ -54,6 +56,7 @@ async function activate(
   factory: IFileBrowserFactory,
   renderMime: IRenderMimeRegistry,
   settingRegistry: ISettingRegistry,
+  docmanager: IDocumentManager,
   statusBar: IStatusBar
 ): Promise<IGitExtension> {
   let gitExtension: GitExtension | null = null;
@@ -106,7 +109,12 @@ async function activate(
     return null;
   }
   // Create the Git model
-  gitExtension = new GitExtension(serverSettings.serverRoot, app, settings);
+  gitExtension = new GitExtension(
+    serverSettings.serverRoot,
+    app,
+    docmanager,
+    settings
+  );
 
   // Whenever we restore the application, sync the Git extension path
   Promise.all([app.restored, filebrowser.model.restored]).then(() => {

src/model.ts

@@ -1,5 +1,6 @@
 import { JupyterFrontEnd } from '@jupyterlab/application';
 import { IChangedArgs, PathExt } from '@jupyterlab/coreutils';
+import { IDocumentManager } from '@jupyterlab/docmanager';
 import { ServerConnection } from '@jupyterlab/services';
 import { ISettingRegistry } from '@jupyterlab/settingregistry';
 import { LinkedList } from '@lumino/collections';
@@ -28,11 +29,13 @@ export class GitExtension implements IGitExtension {
   constructor(
     serverRoot: string,
     app: JupyterFrontEnd = null,
+    docmanager: IDocumentManager = null,
     settings?: ISettingRegistry.ISettings
   ) {
     const self = this;
     this._serverRoot = serverRoot;
     this._app = app;
+    this._docmanager = docmanager;
     this._settings = settings || null;
 
     let interval: number;
@@ -459,6 +462,20 @@ export class GitExtension implements IGitExtension {
     const tid = this._addTask('git:checkout');
     try {
       response = await httpGitRequest('/git/checkout', 'POST', body);
+
+      if (response.ok) {
+        if (body.checkout_branch) {
+          const files = (await this.changedFiles(
+            this._currentBranch.name,
+            body.branchname
+          ))['files'];
+          if (files) {
+            files.forEach(file => this._revertFile(file));
+          }
+        } else {
+          this._revertFile(options.filename);
+        }
+      }
     } catch (err) {
       throw new ServerConnection.NetworkError(err);
     } finally {
@@ -468,6 +485,7 @@ export class GitExtension implements IGitExtension {
     if (!response.ok) {
       throw new ServerConnection.ResponseError(response, data.message);
     }
+
     if (body.checkout_branch) {
       await this.refreshBranch();
       this._headChanged.emit();
@@ -609,11 +627,17 @@ export class GitExtension implements IGitExtension {
       return Promise.resolve(new Response(JSON.stringify(response)));
     }
     const tid = this._addTask('git:commit:revert');
+    const files = (await this.changedFiles(null, null, hash + '^!'))['files'];
     try {
       response = await httpGitRequest('/git/delete_commit', 'POST', {
         commit_id: hash,
         top_repo_path: path
       });
+      if (response.ok && files) {
+        files.forEach(file => {
+          this._revertFile(file);
+        });
+      }
     } catch (err) {
       throw new ServerConnection.NetworkError(err);
     } finally {
@@ -905,12 +929,29 @@ export class GitExtension implements IGitExtension {
       return Promise.resolve(new Response(JSON.stringify(response)));
     }
     const tid = this._addTask('git:reset:changes');
+    const reset_all = filename === undefined;
+    let files;
+    if (reset_all) {
+      files = (await this.changedFiles('INDEX', 'HEAD'))['files'];
+    }
     try {
       response = await httpGitRequest('/git/reset', 'POST', {
         reset_all: filename === undefined,
         filename: filename === undefined ? null : filename,
         top_repo_path: path
       });
+
+      if (response.ok) {
+        if (reset_all) {
+          if (files) {
+            files.forEach(file => {
+              this._revertFile(file);
+            });
+          }
+        } else {
+          this._revertFile(filename);
+        }
+      }
     } catch (err) {
       throw new ServerConnection.NetworkError(err);
     } finally {
@@ -947,12 +988,20 @@ export class GitExtension implements IGitExtension {
       };
       return Promise.resolve(new Response(JSON.stringify(response)));
     }
+    const files = (await this.changedFiles(null, null, hash))['files'];
     const tid = this._addTask('git:reset:hard');
     try {
       response = await httpGitRequest('/git/reset_to_commit', 'POST', {
         commit_id: hash,
         top_repo_path: path
       });
+      if (response.ok) {
+        if (files) {
+          files.forEach(file => {
+            this._revertFile(file);
+          });
+        }
+      }
     } catch (err) {
       throw new ServerConnection.NetworkError(err);
     } finally {
@@ -1226,6 +1275,37 @@ export class GitExtension implements IGitExtension {
     return Promise.resolve(response);
   }
 
+  /**
+   * Get list of files changed between two commits or two branches
+   * @param base id of base commit or base branch for comparison
+   * @param remote id of remote commit or remote branch for comparison
+   * @param singleCommit id of a single commit
+   *
+   * @returns the names of the changed files
+   */
+  async changedFiles(
+    base?: string,
+    remote?: string,
+    singleCommit?: string
+  ): Promise<Git.IChangedFilesResult> {
+    try {
+      const response = await httpGitRequest('/git/changed_files', 'POST', {
+        current_path: this.pathRepository,
+        base: base,
+        remote: remote,
+        single_commit: singleCommit
+      });
+      if (!response.ok) {
+        return response.json().then((data: any) => {
+          throw new ServerConnection.ResponseError(response, data.message);
+        });
+      }
+      return response.json();
+    } catch (err) {
+      throw new ServerConnection.NetworkError(err);
+    }
+  }
+
   /**
    * Make request for a list of all git branches in the repository
    * Retrieve a list of repository branches.
@@ -1344,12 +1424,26 @@ export class GitExtension implements IGitExtension {
     return this._taskID;
   }
 
+  /**
+   * if file is open in JupyterLab find the widget and ensure the JupyterLab
+   * version matches the version on disk. Do nothing if the file has unsaved changes
+   *
+   * @param path path to the file to be reverted
+   */
+  private _revertFile(path: string): void {
+    const widget = this._docmanager.findWidget(this.getRelativeFilePath(path));
+    if (widget && !widget.context.model.dirty) {
+      widget.context.revert();
+    }
+  }
+
   private _status: Git.IStatusFile[] = [];
   private _pathRepository: string | null = null;
   private _branches: Git.IBranch[];
   private _currentBranch: Git.IBranch;
   private _serverRoot: string;
   private _app: JupyterFrontEnd | null;
+  private _docmanager: IDocumentManager | null;
   private _diffProviders: { [key: string]: Git.IDiffCallback } = {};
   private _isDisposed = false;
   private _markerCache: Markers = new Markers(() => this._markChanged.emit());

src/tokens.ts

@@ -451,6 +451,15 @@ export namespace Git {
     files?: IStatusFileResult[];
   }
 
+  /** Interface for changed_files request result
+   * lists the names of files that have differences between two commits
+   * or beween two branches, or that were changed by a single commit
+   */
+  export interface IChangedFilesResult {
+    code: number;
+    files?: string[];
+  }
+
   /** Interface for GitLog request result,
    * has the info of a single past commit
    */