Repository: steveukx/git-js (ts)

Issue:
staged deleted files missing from StatusSummary->staged[]
Trying to figure out if this is a bug or I'm not understanding simple-git's status structure. Given the following `git status` CLI output, I would expect the staged deleted files to appear in the `git.status()` `staged` array property, but they are not.

**`git status`**
```
On branch master
Your branch is ahead of 'origin/master' by 1 commit.
  (use "git push" to publish your local commits)

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   data/db.json
        deleted:    public/assets/16b8dd27-8c71-452d-a33a-bac16dfeb25b-ctyyFSsgG.jpg
        deleted:    public/assets/2bfeff0a-c2fd-4c1f-b7ce-d9aaa85ef9e0-GRyHQR8Wi.jpg
        deleted:    public/assets/rotterdam-_7T4AFdYM.png
        deleted:    public/assets/screen-shot-2020-11-04-at-84705-am-VnSkK4fMa.png
        deleted:    public/assets/screen-shot-2020-11-04-at-85706-am-qs3duZeeN.png
        deleted:    public/assets/screen-shot-2020-11-04-at-90017-am-FSMcon-hf.png
        deleted:    public/assets/screen-shot-2020-11-04-at-94134-am-zsYjqtTbv.png

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   package-lock.json
        modified:   package.json
        modified:   src-backend/api-history.js
        modified:   src-backend/api.js
        modified:   src/views/History.vue
```

**`console.log(git.status())`**
```
StatusSummary {
  not_added: [],
  conflicted: [],
  created: [],
  deleted: [
    'public/assets/16b8dd27-8c71-452d-a33a-bac16dfeb25b-ctyyFSsgG.jpg',
    'public/assets/2bfeff0a-c2fd-4c1f-b7ce-d9aaa85ef9e0-GRyHQR8Wi.jpg',
    'public/assets/rotterdam-_7T4AFdYM.png',
    'public/assets/screen-shot-2020-11-04-at-84705-am-VnSkK4fMa.png',
    'public/assets/screen-shot-2020-11-04-at-85706-am-qs3duZeeN.png',
    'public/assets/screen-shot-2020-11-04-at-90017-am-FSMcon-hf.png',
    'public/assets/screen-shot-2020-11-04-at-94134-am-zsYjqtTbv.png'
  ],
  modified: [
    'data/db.json',
    'package-lock.json',
    'package.json',
    'src-backend/api-history.js',
    'src-backend/api.js',
    'src/views/History.vue'
  ],
  renamed: [],
  files: [
    FileStatusSummary {
      path: 'data/db.json',
      index: 'M',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'package-lock.json',
      index: ' ',
      working_dir: 'M'
    },
    FileStatusSummary {
      path: 'package.json',
      index: ' ',
      working_dir: 'M'
    },
    FileStatusSummary {
      path: 'public/assets/16b8dd27-8c71-452d-a33a-bac16dfeb25b-ctyyFSsgG.jpg',
      index: 'D',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'public/assets/2bfeff0a-c2fd-4c1f-b7ce-d9aaa85ef9e0-GRyHQR8Wi.jpg',
      index: 'D',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'public/assets/rotterdam-_7T4AFdYM.png',
      index: 'D',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'public/assets/screen-shot-2020-11-04-at-84705-am-VnSkK4fMa.png',
      index: 'D',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'public/assets/screen-shot-2020-11-04-at-85706-am-qs3duZeeN.png',
      index: 'D',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'public/assets/screen-shot-2020-11-04-at-90017-am-FSMcon-hf.png',
      index: 'D',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'public/assets/screen-shot-2020-11-04-at-94134-am-zsYjqtTbv.png',
      index: 'D',
      working_dir: ' '
    },
    FileStatusSummary {
      path: 'src-backend/api-history.js',
      index: ' ',
      working_dir: 'M'
    },
    FileStatusSummary {
      path: 'src-backend/api.js',
      index: ' ',
      working_dir: 'M'
    },
    FileStatusSummary {
      path: 'src/views/History.vue',
      index: ' ',
      working_dir: 'M'
    }
  ],
  staged: [ 'data/db.json' ],
  ahead: 1,
  behind: 0,
  current: 'master',
  tracking: 'origin/master'
}
```

Thanks in advance for any explanation.

Pull request description:
Git.status parsing
- Updates the `StatusResult` response to `git.status` to reference staged deletions/modifications in the `staged` array
- Adds support in status parsing for staged rename with unstaged modifications
- Return type in `statusTask` to be interface rather than concrete class

Closes #530

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9f12ebd..3e20abf 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,13 @@
 
 # Change History & Release Notes
 
+## 2.27.0 Included staged delete/modify in StatusResult staged array  
+
+-  Update the `git.status` parser to account for staged deleted/modified files and staged files with subsequent
+   modifications meaning a status of:
+   - `RM old -> new` will now appear in `renamed` and `new` will also appear in `modified`
+   - `D  file` will now appear in both `deleted` and `staged` where ` D file` would only appear in `deleted`
+  
 ## 2.26.0 Fix error when using `git.log` with callback
 
 - Resolves an issue whereby using `git.log` with a callback (or awaiting the promise created from the now deprecated
diff --git a/src/lib/responses/StatusSummary.ts b/src/lib/responses/StatusSummary.ts
index 123a12b..f0bf1f5 100644
--- a/src/lib/responses/StatusSummary.ts
+++ b/src/lib/responses/StatusSummary.ts
@@ -1,5 +1,5 @@
-import { FileStatusResult, StatusResult, StatusResultRenamed } from '../../../typings/response';
-
+import { FileStatusResult, StatusResult, StatusResultRenamed } from '../../../typings';
+import { append } from '../utils';
 import { FileStatusSummary } from './FileStatusSummary';
 
 /**
@@ -43,137 +43,132 @@ export class StatusSummary implements StatusResult {
    /**
     * Gets whether this StatusSummary represents a clean working branch.
     */
-   public isClean (): boolean {
+   public isClean(): boolean {
       return !this.files.length;
    }
 }
 
-export type StatusSummaryParserFn = {
-   (line: string, status: StatusSummary, indexState: string, workingDir: string): void;
+enum PorcelainFileStatus {
+   ADDED = 'A',
+   DELETED = 'D',
+   MODIFIED = 'M',
+   RENAMED = 'R',
+   COPIED = 'C',
+   UNMERGED = 'U',
+   UNTRACKED = '?',
+   IGNORED = '!',
+   NONE = ' ',
 }
 
-export const StatusSummaryParsers: {[key: string]: StatusSummaryParserFn} = {
-   '##': function (line, status) {
-      const aheadReg = /ahead (\d+)/;
-      const behindReg = /behind (\d+)/;
-      const currentReg = /^(.+?(?=(?:\.{3}|\s|$)))/;
-      const trackingReg = /\.{3}(\S*)/;
-      const onEmptyBranchReg = /\son\s([\S]+)$/;
-      let regexResult;
+function renamedFile(line: string) {
+   const detail = /^(.+) -> (.+)$/.exec(line);
 
-      regexResult = aheadReg.exec(line);
-      status.ahead = regexResult && +regexResult[1] || 0;
+   if (!detail) {
+      return {
+         from: line, to: line
+      };
+   }
 
-      regexResult = behindReg.exec(line);
-      status.behind = regexResult && +regexResult[1] || 0;
+   return {
+      from: String(detail[1]),
+      to: String(detail[2]),
+   };
+}
 
-      regexResult = currentReg.exec(line);
-      status.current = regexResult && regexResult[1];
+function parser(indexX: PorcelainFileStatus, indexY: PorcelainFileStatus, handler: (result: StatusSummary, file: string) => void): [string, (result: StatusSummary, file: string) => unknown] {
+   return [`${indexX}${indexY}`, handler];
+}
 
-      regexResult = trackingReg.exec(line);
-      status.tracking = regexResult && regexResult[1];
+function conflicts(indexX: PorcelainFileStatus, ...indexY: PorcelainFileStatus[]) {
+   return indexY.map(y => parser(indexX, y, (result, file) => append(result.conflicted, file)));
+}
 
-      regexResult = onEmptyBranchReg.exec(line);
-      status.current = regexResult && regexResult[1] || status.current;
-   },
+const parsers: Map<string, (result: StatusSummary, file: string) => unknown> = new Map([
+   parser(PorcelainFileStatus.NONE, PorcelainFileStatus.ADDED, (result, file) => append(result.created, file)),
+   parser(PorcelainFileStatus.NONE, PorcelainFileStatus.DELETED, (result, file) => append(result.deleted, file)),
+   parser(PorcelainFileStatus.NONE, PorcelainFileStatus.MODIFIED, (result, file) => append(result.modified, file)),
 
-   '??': function (line, status) {
-      status.not_added.push(line);
-   },
+   parser(PorcelainFileStatus.ADDED, PorcelainFileStatus.NONE, (result, file) => append(result.created, file) && append(result.staged, file)),
+   parser(PorcelainFileStatus.ADDED, PorcelainFileStatus.MODIFIED, (result, file) =>
+      append(result.created, file) && append(result.staged, file) && append(result.modified, file)),
 
-   A: function (line, status) {
-      status.created.push(line);
-   },
+   parser(PorcelainFileStatus.DELETED, PorcelainFileStatus.NONE, (result, file) => append(result.deleted, file) && append(result.staged, file)),
 
-   AM: function (line, status) {
-      status.created.push(line);
-   },
+   parser(PorcelainFileStatus.MODIFIED, PorcelainFileStatus.NONE, (result, file) => append(result.modified, file) && append(result.staged, file)),
+   parser(PorcelainFileStatus.MODIFIED, PorcelainFileStatus.MODIFIED, (result, file) => append(result.modified, file) && append(result.staged, file)),
 
-   D: function (line, status) {
-      status.deleted.push(line);
-   },
+   parser(PorcelainFileStatus.RENAMED, PorcelainFileStatus.NONE, (result, file) => {
+      append(result.renamed, renamedFile(file));
+   }),
+   parser(PorcelainFileStatus.RENAMED, PorcelainFileStatus.MODIFIED, (result, file) => {
+      const renamed = renamedFile(file);
+      append(result.renamed, renamed);
+      append(result.modified, renamed.to);
+   }),
 
-   M: function (line, status, indexState) {
-      status.modified.push(line);
+   parser(PorcelainFileStatus.UNTRACKED, PorcelainFileStatus.UNTRACKED, (result, file) => append(result.not_added, file)),
 
-      if (indexState === 'M') {
-         status.staged.push(line);
-      }
-   },
+   ...conflicts(PorcelainFileStatus.ADDED, PorcelainFileStatus.ADDED, PorcelainFileStatus.UNMERGED),
+   ...conflicts(PorcelainFileStatus.DELETED, PorcelainFileStatus.DELETED, PorcelainFileStatus.UNMERGED),
+   ...conflicts(PorcelainFileStatus.UNMERGED, PorcelainFileStatus.ADDED, PorcelainFileStatus.DELETED, PorcelainFileStatus.UNMERGED),
+
+   ['##', (result, line) => {
+      const aheadReg = /ahead (\d+)/;
+      const behindReg = /behind (\d+)/;
+      const currentReg = /^(.+?(?=(?:\.{3}|\s|$)))/;
+      const trackingReg = /\.{3}(\S*)/;
+      const onEmptyBranchReg = /\son\s([\S]+)$/;
+      let regexResult;
 
-   R: function (line, status) {
-      const detail = /^(.+) -> (.+)$/.exec(line) || [null, line, line];
+      regexResult = aheadReg.exec(line);
+      result.ahead = regexResult && +regexResult[1] || 0;
 
-      status.renamed.push({
-         from: String(detail[1]),
-         to: String(detail[2])
-      });
-   },
+      regexResult = behindReg.exec(line);
+      result.behind = regexResult && +regexResult[1] || 0;
 
-   UU: function (line, status) {
-      status.conflicted.push(line);
-   }
-};
+      regexResult = currentReg.exec(line);
+      result.current = regexResult && regexResult[1];
 
-StatusSummaryParsers.MM = StatusSummaryParsers.M;
+      regexResult = trackingReg.exec(line);
+      result.tracking = regexResult && regexResult[1];
 
-/* Map all unmerged status code combinations to UU to mark as conflicted */
-StatusSummaryParsers.AA = StatusSummaryParsers.UU;
-StatusSummaryParsers.UD = StatusSummaryParsers.UU;
-StatusSummaryParsers.DU = StatusSummaryParsers.UU;
-StatusSummaryParsers.DD = StatusSummaryParsers.UU;
-StatusSummaryParsers.AU = StatusSummaryParsers.UU;
-StatusSummaryParsers.UA = StatusSummaryParsers.UU;
+      regexResult = onEmptyBranchReg.exec(line);
+      result.current = regexResult && regexResult[1] || result.current;
+   }]
+]);
 
-export const parseStatusSummary = function (text: string): StatusSummary {
-   let file;
+export const parseStatusSummary = function (text: string): StatusResult {
    const lines = text.trim().split('\n');
    const status = new StatusSummary();
 
    for (let i = 0, l = lines.length; i < l; i++) {
-      file = splitLine(lines[i]);
-
-      if (!file) {
-         continue;
-      }
-
-      if (file.handler) {
-         file.handler(file.path, status, file.index, file.workingDir);
-      }
-
-      if (file.code !== '##') {
-         status.files.push(new FileStatusSummary(file.path, file.index, file.workingDir));
-      }
+      splitLine(status, lines[i]);
    }
 
    return status;
 };
 
-
-function splitLine(lineStr: string) {
-   let line = lineStr.trim().match(/(..?)(\s+)(.*)/);
-   if (!line || !line[1].trim()) {
-      line = lineStr.trim().match(/(..?)\s+(.*)/);
+function splitLine(result: StatusResult, lineStr: string) {
+   const trimmed = lineStr.trim();
+   switch (' ') {
+      case trimmed.charAt(2):
+         return data(trimmed.charAt(0), trimmed.charAt(1), trimmed.substr(3));
+      case trimmed.charAt(1):
+         return data(PorcelainFileStatus.NONE, trimmed.charAt(0), trimmed.substr(2));
+      default:
+         return;
    }
 
-   if (!line) {
-      return;
-   }
+   function data(index: string, workingDir: string, path: string) {
+      const raw = `${index}${workingDir}`;
+      const handler = parsers.get(raw);
 
-   let code = line[1];
-   if (line[2].length > 1) {
-      code += ' ';
-   }
-   if (code.length === 1 && line[2].length === 1) {
-      code = ' ' + code;
-   }
+      if (handler) {
+         handler(result, path);
+      }
 
-   return {
-      raw: code,
-      code: code.trim(),
-      index: code.charAt(0),
-      workingDir: code.charAt(1),
-      handler: StatusSummaryParsers[code.trim()],
-      path: line[3]
-   };
+      if (raw !== '##') {
+         result.files.push(new FileStatusSummary(path, index, workingDir));
+      }
+   }
 }
diff --git a/src/lib/tasks/status.ts b/src/lib/tasks/status.ts
index 3722d46..a01ed0e 100644
--- a/src/lib/tasks/status.ts
+++ b/src/lib/tasks/status.ts
@@ -1,7 +1,8 @@
+import { StatusResult } from '../../../typings';
+import { parseStatusSummary } from '../responses/StatusSummary';
 import { StringTask } from '../types';
-import { parseStatusSummary, StatusSummary } from '../responses/StatusSummary';
 
-export function statusTask(customArgs: string[]): StringTask<StatusSummary> {
+export function statusTask(customArgs: string[]): StringTask<StatusResult> {
    return {
       format: 'utf-8',
       commands: ['status', '--porcelain', '-b', '-u', ...customArgs],


Test patch:
diff --git a/test/unit/__fixtures__/index.ts b/test/unit/__fixtures__/index.ts
index b262db2..e7a522a 100644
--- a/test/unit/__fixtures__/index.ts
+++ b/test/unit/__fixtures__/index.ts
@@ -6,6 +6,7 @@ export * from './instance';
 export * from './responses/diff';
 export * from './responses/merge';
 export * from './responses/remote-messages';
+export * from './responses/status';
 
 export * from '../../__fixtures__/expectations'
 export * from '../../__fixtures__/like'
diff --git a/test/unit/__fixtures__/responses/status.ts b/test/unit/__fixtures__/responses/status.ts
new file mode 100644
index 0000000..bc49019
--- /dev/null
+++ b/test/unit/__fixtures__/responses/status.ts
@@ -0,0 +1,31 @@
+import { createFixture } from '../create-fixture';
+
+export function stagedRenamed(from = 'from.ext', to = 'to.ext', workingDir = ' ') {
+   return `R${workingDir} ${from} -> ${to}`;
+}
+
+export function stagedRenamedWithModifications(from = 'from.ext', to = 'to.ext') {
+   return stagedRenamed(from, to, 'M');
+}
+
+export function stagedDeleted(file = 'staged-deleted.ext') {
+   return `D  ${file}`;
+}
+
+export function unStagedDeleted(file = 'un-staged-deleted.ext') {
+   return ` D ${file}`;
+}
+
+export function stagedModified(file = 'staged-modified.ext') {
+   return `M  ${file}`;
+}
+
+export function statusResponse(branch = 'main', ...files: Array<string | (() => string)>) {
+   const stdOut: string[] = [
+      `## ${branch}`,
+      ...files.map(file => typeof file === 'function' ? file() : file),
+   ];
+
+   return createFixture(stdOut.join('\n'), '');
+
+}
diff --git a/test/unit/status.spec.ts b/test/unit/status.spec.ts
index a691544..e4675e7 100644
--- a/test/unit/status.spec.ts
+++ b/test/unit/status.spec.ts
@@ -4,8 +4,15 @@ import {
    assertGitError,
    closeWithError,
    closeWithSuccess,
+   like,
    newSimpleGit,
-   newSimpleGitP
+   newSimpleGitP,
+   stagedDeleted,
+   stagedModified,
+   stagedRenamed,
+   stagedRenamedWithModifications,
+   statusResponse,
+   unStagedDeleted
 } from './__fixtures__';
 import { SimpleGit, StatusResult } from '../../typings';
 import { parseStatusSummary, StatusSummary } from '../../src/lib/responses/StatusSummary';
@@ -117,6 +124,67 @@ describe('status', () => {
    });
 
    describe('parsing', () => {
+      const empty = {
+         created: [],
+         modified: [],
+         deleted: [],
+         staged: [],
+      };
+
+      it('Ignores unknown modifiers', () => {
+         expect(parseStatusSummary('$@ UNKNOWN')).toEqual(like({
+            ...empty,
+            files: [
+               {
+                  index: '$',
+                  working_dir: '@',
+                  path: 'UNKNOWN',
+               }
+            ],
+         }))
+      })
+
+      it('Handles malformatted rename', () => {
+         expect(parseStatusSummary(statusResponse('main', 'R  file.ext').stdOut)).toEqual(like({
+            ...empty,
+            current: 'main',
+            renamed: [{from: 'file.ext', to: 'file.ext'}],
+         }));
+      });
+
+      it('Handles staged rename', () => {
+         expect(parseStatusSummary(statusResponse('main', stagedRenamed).stdOut)).toEqual(like({
+            ...empty,
+            current: 'main',
+            renamed: [{from: 'from.ext', to: 'to.ext'}],
+         }));
+      });
+
+      it('Handles staged rename with un-staged modifications', () => {
+         expect(parseStatusSummary(statusResponse('main', stagedRenamedWithModifications).stdOut)).toEqual(like({
+            ...empty,
+            current: 'main',
+            renamed: [{from: 'from.ext', to: 'to.ext'}],
+            modified: ['to.ext'],
+         }));
+      });
+
+      it('Handles staged modified', () => {
+         expect(parseStatusSummary(statusResponse('main', stagedModified).stdOut)).toEqual(like({
+            current: 'main',
+            modified: ['staged-modified.ext'],
+         }))
+      });
+
+      it('Handles (un)staged deleted', () => {
+         expect(parseStatusSummary(statusResponse('main', stagedDeleted, unStagedDeleted).stdOut)).toEqual(like({
+            current: 'main',
+            created: [],
+            modified: [],
+            deleted: ['staged-deleted.ext', 'un-staged-deleted.ext'],
+            staged: ['staged-deleted.ext']
+         }))
+      });
 
       it('Initial repo with no commits', () => {
          const statusSummary = parseStatusSummary(`
@@ -149,40 +217,59 @@ R  src/a.txt -> src/c.txt
          }));
       });
 
-      it('parses status', () => {
+      it('parses status - current, tracking and ahead', () => {
          expect(parseStatusSummary('## master...origin/master [ahead 3]')).toEqual(expect.objectContaining({
             current: 'master',
             tracking: 'origin/master',
             ahead: 3,
             behind: 0,
          }));
+      });
 
+      it('parses status - current, tracking and behind', () => {
+         expect(parseStatusSummary('## master...origin/master [behind 2]')).toEqual(expect.objectContaining({
+            current: 'master',
+            tracking: 'origin/master',
+            ahead: 0,
+            behind: 2,
+         }));
+      });
+
+      it('parses status - current, tracking', () => {
          expect(parseStatusSummary('## release/0.34.0...origin/release/0.34.0')).toEqual(expect.objectContaining({
             current: 'release/0.34.0',
             tracking: 'origin/release/0.34.0',
             ahead: 0,
             behind: 0,
          }));
+      });
 
+      it('parses status - HEAD no branch', () => {
          expect(parseStatusSummary('## HEAD (no branch)')).toEqual(expect.objectContaining({
             current: 'HEAD',
             tracking: null,
             ahead: 0,
             behind: 0,
          }));
+      });
 
+      it('parses status - with untracked', () => {
          expect(parseStatusSummary('?? Not tracked File\nUU Conflicted\n D Removed')).toEqual(expect.objectContaining({
             not_added: ['Not tracked File'],
             conflicted: ['Conflicted'],
             deleted: ['Removed'],
          }));
+      });
 
+      it('parses status - modified, added and added-changed', () => {
          expect(parseStatusSummary(' M Modified\n A Added\nAM Changed')).toEqual(expect.objectContaining({
-            modified: ['Modified'],
+            modified: ['Modified', 'Changed'],
             created: ['Added', 'Changed'],
          }));
+      });
 
-         expect(parseStatusSummary('## this_branch')).toEqual(expect.objectContaining({
+      it('parses status', () => {
+         expect(parseStatusSummary(statusResponse('this_branch').stdOut)).toEqual(expect.objectContaining({
             current: 'this_branch',
             tracking: null,
          }));
@@ -214,7 +301,7 @@ R  src/a.txt -> src/c.txt
             ?? ddd
       `);
          expect(statusSummary).toEqual(expect.objectContaining({
-            staged: ['bbb'],
+            staged: ['bbb', 'ccc'],
             modified: ['aaa', 'bbb'],
          }));
       });
@@ -238,18 +325,17 @@ R  src/a.txt -> src/c.txt
             M  src/git.js
             AM src/index.js
              A src/newfile.js
-            AM test.js
             ?? test
             UU test.js
       `);
 
-         expect(statusSummary).toEqual(expect.objectContaining({
-            created: ['src/index.js', 'src/newfile.js', 'test.js'],
+         expect(statusSummary).toEqual(like({
+            created: ['src/index.js', 'src/newfile.js'],
             deleted: [],
-            modified: ['package.json', 'src/git.js'],
+            modified: ['package.json', 'src/git.js', 'src/index.js'],
             not_added: ['test'],
             conflicted: ['test.js'],
-            staged: ['src/git.js'],
+            staged: ['src/git.js', 'src/index.js'],
          }));
       });