Fix stash REST API (#1280)

* WIP parse stash on the backend

* Parse stash on the backend

* Remove duplicate workflow

* Update frontend to new REST API

* Fix typing for stash index

* Lint the code

* Fix index parsed on the backend

* Improve test robustness

Author: fcollonval

.github/workflows/pre-release.yml

@@ -46,6 +46,10 @@
 GIT_REBASING_BRANCH = re.compile(r"^\(no branch, rebasing (?P<branch>.+?)\)$")
 # Git cache as a credential helper
 GIT_CREDENTIAL_HELPER_CACHE = re.compile(r"cache\b")
+# Parse git stash list
+GIT_STASH_LIST = re.compile(
+    r"^stash@{(?P<index>\d+)}: (WIP on|On) (?P<branch>.+?): (?P<message>.+?)$"
+)
 
 execution_lock = tornado.locks.Lock()
 
@@ -1985,7 +1989,8 @@ async def stash(self, path: str, stashMsg: str = "") -> dict:
         # code 0: no changes to stash
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": error}
-        return {"code": code, "message": output, "command": " ".join(cmd)}
+
+        return {"code": code, "message": output.strip()}
 
     async def stash_list(self, path: str) -> dict:
         """
@@ -2001,7 +2006,15 @@ async def stash_list(self, path: str) -> dict:
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": error}
 
-        return {"code": code, "message": output, "command": " ".join(cmd)}
+        stashes = []
+        for line in output.strip("\n").splitlines():
+            match = GIT_STASH_LIST.match(line)
+            if match is not None:
+                d = match.groupdict()
+                d["index"] = int(d["index"])
+                stashes.append(d)
+
+        return {"code": code, "stashes": stashes}
 
     async def stash_show(self, path: str, index: int) -> dict:
         """
@@ -2020,7 +2033,9 @@ async def stash_show(self, path: str, index: int) -> dict:
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": error}
 
-        return {"code": code, "message": output, "command": " ".join(cmd)}
+        files = output.strip("\n").splitlines()
+
+        return {"code": code, "files": files}
 
     async def pop_stash(self, path: str, stash_index: Optional[int] = None) -> dict:
         """
@@ -2045,7 +2060,7 @@ async def pop_stash(self, path: str, stash_index: Optional[int] = None) -> dict:
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": error}
 
-        return {"code": code, "message": output, "command": " ".join(cmd)}
+        return {"code": code, "message": output.strip()}
 
     async def drop_stash(self, path, stash_index: Optional[int] = None) -> dict:
         """
@@ -2071,7 +2086,7 @@ async def drop_stash(self, path, stash_index: Optional[int] = None) -> dict:
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": error}
 
-        return {"code": code, "message": output, "command": " ".join(cmd)}
+        return {"code": code, "message": output.strip()}
 
     async def apply_stash(self, path: str, stash_index: Optional[int] = None) -> dict:
         """
@@ -2098,7 +2113,7 @@ async def apply_stash(self, path: str, stash_index: Optional[int] = None) -> dic
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": error}
 
-        return {"code": code, "message": output, "command": " ".join(cmd)}
+        return {"code": code, "message": output.strip()}
 
     @property
     def excluded_paths(self) -> List[str]:

jupyterlab_git/git.py

@@ -964,8 +964,7 @@ async def post(self, path: str = "", stashMsg: str = ""):
             self.set_status(201)
         else:
             self.set_status(500)
-
-        self.finish(json.dumps(response))
+            self.finish(json.dumps(response))
 
     @tornado.web.authenticated
     async def delete(self, path: str = ""):
@@ -983,9 +982,10 @@ async def delete(self, path: str = ""):
 
         if response["code"] == 0:
             self.set_status(204)
+            self.finish()
         else:
             self.set_status(500)
-        self.finish()
+            self.finish(json.dumps(response))
 
     @tornado.web.authenticated
     async def get(self, path: str = ""):
@@ -1048,8 +1048,7 @@ async def post(self, path: str = ""):
             self.set_status(201)
         else:
             self.set_status(500)
-
-        self.finish(json.dumps(response))
+            self.finish(json.dumps(response))
 
 
 def setup_handlers(web_app):

jupyterlab_git/handlers.py

@@ -42,13 +42,6 @@ async def test_git_stash_without_message(mock_execute, jp_fetch, jp_root_dir):
     )
 
     assert response.code == 201
-    # json.loads turns a string into a dictionary
-    payload = json.loads(response.body)
-    assert payload == {
-        "code": 0,
-        "message": "",
-        "command": " ".join(command),
-    }
 
 
 @patch("jupyterlab_git.git.execute")
@@ -115,12 +108,6 @@ async def test_git_stash_with_message(mock_execute, jp_fetch, jp_root_dir):
     )
 
     assert response.code == 201
-    payload = json.loads(response.body)
-    assert payload == {
-        "code": 0,
-        "message": "",
-        "command": " ".join(command),
-    }
 
 
 # Git Stash - GET
@@ -134,7 +121,13 @@ async def test_git_stash_show_with_index(mock_execute, jp_fetch, jp_root_dir):
     local_path = jp_root_dir / "test_path"
     stash_index = 1
 
-    mock_execute.return_value = maybe_future((0, "", ""))
+    mock_execute.return_value = maybe_future(
+        (
+            0,
+            "node_modules/playwright-core/README.md\nnode_modules/playwright-core/package.json\n",
+            "",
+        )
+    )
 
     # When
     response = await jp_fetch(
@@ -164,8 +157,10 @@ async def test_git_stash_show_with_index(mock_execute, jp_fetch, jp_root_dir):
     payload = json.loads(response.body)
     assert payload == {
         "code": 0,
-        "message": "",
-        "command": " ".join(command),
+        "files": [
+            "node_modules/playwright-core/README.md",
+            "node_modules/playwright-core/package.json",
+        ],
     }
 
 
@@ -176,7 +171,13 @@ async def test_git_stash_show_without_index(mock_execute, jp_fetch, jp_root_dir)
     env["GIT_TERMINAL_PROMPT"] = "0"
     local_path = jp_root_dir / "test_path"
 
-    mock_execute.return_value = maybe_future((0, "", ""))
+    mock_execute.return_value = maybe_future(
+        (
+            0,
+            "stash@{0}: On main: testsd\nstash@{1}: WIP on main: bea6895 first commit\n",
+            "",
+        )
+    )
 
     # When
     response = await jp_fetch(
@@ -202,8 +203,10 @@ async def test_git_stash_show_without_index(mock_execute, jp_fetch, jp_root_dir)
     payload = json.loads(response.body)
     assert payload == {
         "code": 0,
-        "message": "",
-        "command": " ".join(command),
+        "stashes": [
+            {"index": 0, "branch": "main", "message": "testsd"},
+            {"index": 1, "branch": "main", "message": "bea6895 first commit"},
+        ],
     }
 
 

jupyterlab_git/tests/test_stash.py

@@ -154,7 +154,7 @@ export interface IGitPanelState {
    * Stashed files
    *
    */
-  stash: Git.IStash;
+  stash: Git.IStash[];
 }
 
 /**

src/components/GitPanel.tsx

@@ -47,7 +47,8 @@ export interface IGitStashProps {
   /**
    * Files in the group
    */
-  stash: Git.IStash;
+  stash: Git.IStash[];
+
   /**
    * HTML element height
    */

src/components/GitStash.tsx

@@ -266,14 +266,14 @@ export class GitExtension implements IGitExtension {
    *  A signal emitted when the Git stash changes.
    *
    */
-  get stashChanged(): ISignal<IGitExtension, IChangedArgs<Git.IStash>> {
+  get stashChanged(): ISignal<IGitExtension, IChangedArgs<Git.IStash[]>> {
     return this._stashChanged;
   }
 
   /**
    * The repository stash
    */
-  get stash(): Git.IStash {
+  get stash(): Git.IStash[] {
     return this._stash;
   }
 
@@ -1655,64 +1655,36 @@ export class GitExtension implements IGitExtension {
 
     // Get the entire stash list
     try {
-      const stashListData =
-        await this._taskHandler.execute<Git.IStashListResult>(
-          'git:refresh:stash',
-          async () => {
-            return await requestAPI<Git.IStashListResult>(
-              URLExt.join(path, 'stash'),
-              'GET'
-            );
-          }
-        );
-
-      // Contains the raw message
-      const stashMsgList = stashListData.message.split('\n').slice(0, -1);
-
-      const stashList: Git.IStashEntry[] = [];
-      for (const index in stashMsgList) {
-        const stashInfo = stashMsgList[index].split(':');
-
-        const WIPMatch = stashInfo[1].match(/WIP on (.*)/);
-        const branchName = WIPMatch ? WIPMatch[1] : stashInfo[1].split(' ')[2];
-
-        const stashMsg = stashInfo[2].replace(/^\s+/, '');
-
-        stashList.push({
-          index: parseInt(index, 10),
-          branch: branchName,
-          message: stashMsg,
-          files: []
-        });
-      }
-      const fileData = await this._taskHandler.execute<Git.IStashListResult>(
+      const response = await this._taskHandler.execute<Git.IStashListResult>(
         'git:refresh:stash',
         async () => {
-          const results = await Promise.all(
-            stashList.map(({ index }) =>
-              requestAPI<Git.IStashListResult>(
-                URLExt.join(path, 'stash') + `?index=${index}`,
-                'GET'
-              )
-            )
+          return await requestAPI<Git.IStashListResult>(
+            URLExt.join(path, 'stash'),
+            'GET'
           );
-          return {
-            message: 'Stash list result',
-            command: 'git:refresh:stash',
-            code: 0,
-            results
-          };
         }
       );
 
-      stashList.forEach((stash, index) => {
-        stash.files.push(
-          ...(fileData.results ?? [])[index].message.split('\n').slice(0, -1)
-        );
-      });
+      const allStashFiles = await this._taskHandler.execute<
+        Git.IStashShowResult[]
+      >('git:refresh:stash', () =>
+        Promise.all(
+          response.stashes.map(({ index }) =>
+            requestAPI<Git.IStashShowResult>(
+              URLExt.join(path, 'stash') + `?index=${index}`,
+              'GET'
+            )
+          )
+        )
+      );
+      const stashList: Git.IStash[] = response.stashes.map((s, index) =>
+        Object.assign(s, {
+          files: allStashFiles[index].files
+        })
+      );
 
       if (!this.isStashDeepEqual(stashList, this._stash)) {
-        const change: IChangedArgs<Git.IStash> = {
+        const change: IChangedArgs<Git.IStash[]> = {
           name: 'stash',
           newValue: stashList,
           oldValue: this._stash
@@ -1772,10 +1744,7 @@ export class GitExtension implements IGitExtension {
    * @throws {Git.GitResponseError} If the server response is not ok
    * @throws {ServerConnection.NetworkError} If the request cannot be made
    */
-  protected isStashDeepEqual(
-    a: Git.IStashEntry[],
-    b: Git.IStashEntry[]
-  ): boolean {
+  protected isStashDeepEqual(a: Git.IStash[], b: Git.IStash[]): boolean {
     if (a?.length !== b?.length) {
       return false;
     }
@@ -2212,7 +2181,7 @@ export class GitExtension implements IGitExtension {
     state: Git.State.DEFAULT,
     files: []
   };
-  private _stash: Git.IStash = [];
+  private _stash: Git.IStash[] = [];
   private _pathRepository: string | null = null;
   private _branches: Git.IBranch[] = [];
   private _tagsList: Git.ITag[] = [];
@@ -2251,7 +2220,7 @@ export class GitExtension implements IGitExtension {
     IGitExtension,
     IChangedArgs<string | null>
   >(this);
-  private _stashChanged = new Signal<IGitExtension, IChangedArgs<Git.IStash>>(
+  private _stashChanged = new Signal<IGitExtension, IChangedArgs<Git.IStash[]>>(
     this
   );
   private _statusChanged = new Signal<IGitExtension, Git.IStatus>(this);