Merge pull request #771 from steveukx/feat/status-with-nulls

steveukx · web-flow · commit 357b4deb679d · 2022-03-18T15:46:53.000Z
Status Summary should use null terminators to allow files with spaces…
diff --git a/.changeset/sixty-rivers-drop.md b/.changeset/sixty-rivers-drop.md
@@ -0,0 +1,5 @@
+---
+"simple-git": minor
+---
+
+Use null separators in git.status to allow for non-ascii file names
diff --git a/simple-git/src/lib/responses/StatusSummary.ts b/simple-git/src/lib/responses/StatusSummary.ts
@@ -1,5 +1,5 @@
 import { StatusResult } from '../../../typings';
-import { append } from '../utils';
+import { append, NULL } from '../utils';
 import { FileStatusSummary } from './FileStatusSummary';
 
 type StatusLineParser = (result: StatusResult, file: string) => void;
@@ -120,7 +120,7 @@ const parsers: Map<string, StatusLineParser> = new Map([
 ]);
 
 export const parseStatusSummary = function (text: string): StatusResult {
-   const lines = text.trim().split('\n');
+   const lines = text.trim().split(NULL);
    const status = new StatusSummary();
 
    for (let i = 0, l = lines.length; i < l; i++) {
diff --git a/simple-git/src/lib/tasks/status.ts b/simple-git/src/lib/tasks/status.ts
@@ -2,10 +2,21 @@ import { StatusResult } from '../../../typings';
 import { parseStatusSummary } from '../responses/StatusSummary';
 import { StringTask } from '../types';
 
+const ignoredOptions = ['--null', '-z'];
+
 export function statusTask(customArgs: string[]): StringTask<StatusResult> {
+   const commands = [
+      'status',
+      '--porcelain',
+      '-b',
+      '-u',
+      '--null',
+      ...customArgs.filter(arg => !ignoredOptions.includes(arg))
+   ];
+
    return {
       format: 'utf-8',
-      commands: ['status', '--porcelain', '-b', '-u', ...customArgs],
+      commands,
       parser(text: string) {
          return parseStatusSummary(text);
       }
diff --git a/simple-git/test/unit/__fixtures__/responses/status.ts b/simple-git/test/unit/__fixtures__/responses/status.ts
@@ -1,4 +1,5 @@
 import { createFixture } from '../create-fixture';
+import { NULL } from '../../../../src/lib/utils';
 
 export function stagedRenamed(from = 'from.ext', to = 'to.ext', workingDir = ' ') {
    return `R${workingDir} ${from} -> ${to}`;
@@ -30,6 +31,6 @@ export function statusResponse(branch = 'main', ...files: Array<string | (() =>
       ...files.map(file => typeof file === 'function' ? file() : file),
    ];
 
-   return createFixture(stdOut.join('\n'), '');
+   return createFixture(stdOut.join(NULL), '');
 
 }
diff --git a/simple-git/test/unit/status.spec.ts b/simple-git/test/unit/status.spec.ts
@@ -17,11 +17,12 @@ import {
 } from './__fixtures__';
 import { SimpleGit, StatusResult } from '../../typings';
 import { parseStatusSummary, StatusSummary } from '../../src/lib/responses/StatusSummary';
+import { NULL } from '../../src/lib/utils';
 
 describe('status', () => {
    let git: SimpleGit;
    let callback: jest.Mock;
-   let statusCommands = (...extras: string[]) => ['status', '--porcelain', '-b', '-u', ...extras];
+   let statusCommands = (...extras: string[]) => ['status', '--porcelain', '-b', '-u', '--null', ...extras];
 
    beforeEach(() => callback = jest.fn());
 
@@ -62,6 +63,13 @@ describe('status', () => {
 
       beforeEach(() => git = newSimpleGit());
 
+      it('ignores explicit --null option', async () => {
+         git.status(['-a', '--null', '-b', '-z', '-c']);
+         await closeWithSuccess();
+
+         assertExecutedCommands(...statusCommands('-a', '-b', '-c'));
+      });
+
       it('throws errors to the rejection handler', async () => {
          const queue = git.status();
          await closeWithError('unknown');
@@ -145,6 +153,20 @@ describe('status', () => {
          }))
       });
 
+      it('Handles files with non ascii names', () => {
+         expect(parseStatusSummary(statusResponse('main', stagedModified('😀 file.ext')).stdOut)).toEqual(like({
+            current: 'main',
+            modified: ['😀 file.ext'],
+         }));
+      });
+
+      it('Handles files with spaces in their names', () => {
+         expect(parseStatusSummary(statusResponse('main', stagedModified('foo bar.ext')).stdOut)).toEqual(like({
+            current: 'main',
+            modified: ['foo bar.ext'],
+         }));
+      });
+
       it('Handles ignored files', () => {
          expect(parseStatusSummary(statusResponse('main', stagedIgnored).stdOut)).toEqual(like({
             ...empty,
@@ -196,22 +218,18 @@ describe('status', () => {
       });
 
       it('Initial repo with no commits', () => {
-         const statusSummary = parseStatusSummary(`
-## No commits yet on master
-         `);
+         const statusSummary = parseStatusSummary(`## No commits yet on master`);
 
          expect(statusSummary).toEqual(like({
             current: `master`
          }))
       });
 
       it('Complex status - renamed, new and un-tracked modifications', () => {
-         const statusSummary = parseStatusSummary(`
-## master
- M other.txt
-A  src/b.txt
-R  src/a.txt -> src/c.txt
-`);
+         const statusSummary = parseStatusSummary(statusResponse('master',
+            ' M other.txt',
+            'A  src/b.txt',
+            'R  src/a.txt -> src/c.txt').stdOut);
 
          expect(statusSummary).toEqual(like({
             created: ['src/b.txt'],
@@ -264,20 +282,23 @@ R  src/a.txt -> src/c.txt
          }));
       });
 
-      it('parses status - with untracked', () => {
-         expect(parseStatusSummary('?? Not tracked File\nUU Conflicted\n D Removed')).toEqual(like({
-            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(like({
-            modified: ['Modified', 'Changed'],
-            created: ['Added', 'Changed'],
-         }));
-      });
+      it.each<[string, any]>([
+         ['?? Not tracked File', { not_added: ['Not tracked File'] }],
+         ['UU Conflicted', { conflicted: ['Conflicted'] }],
+         [' D Removed', { deleted: ['Removed'] }],
+         [' M Modified', { modified: ['Modified'] }],
+         [' A Added', { created: ['Added'] }],
+         ['AM Changed', { created: ['Changed'], modified: ['Changed'] }],
+      ])('parses file status - %s', (file, result) => {
+         expect(parseStatusSummary(statusResponse('branch', file).stdOut)).toEqual(like({
+            modified: [],
+            created: [],
+            not_added: [],
+            conflicted: [],
+            deleted: [],
+            ...result,
+         }))
+      })
 
       it('parses status', () => {
          expect(parseStatusSummary(statusResponse('this_branch').stdOut)).toEqual(like({
@@ -291,7 +312,7 @@ R  src/a.txt -> src/c.txt
       });
 
       it('allows isClean to be destructured', () => {
-         const { isClean } = parseStatusSummary('\n');
+         const {isClean} = parseStatusSummary('\n');
          expect(isClean()).toBe(true);
       });
 
@@ -309,41 +330,23 @@ R  src/a.txt -> src/c.txt
       });
 
       it('staged modified files identified separately to other modified files', () => {
-         const statusSummary = parseStatusSummary(`
-            ## master
-             M aaa
-            M  bbb
-            A  ccc
-            ?? ddd
-      `);
+         const statusSummary = parseStatusSummary(`## master${NULL} M aaa${NULL}M  bbb${NULL}A  ccc${NULL}?? ddd`);
          expect(statusSummary).toEqual(like({
             staged: ['bbb', 'ccc'],
             modified: ['aaa', 'bbb'],
          }));
       });
 
       it('staged modified file with modifications after staging', () => {
-         const statusSummary = parseStatusSummary(`
-            ## master
-            MM staged-modified
-             M modified
-            M  staged
-      `);
+         const statusSummary = parseStatusSummary(`## master${NULL}MM staged-modified${NULL} M modified${NULL}M  staged`);
          expect(statusSummary).toEqual(like({
             staged: ['staged-modified', 'staged'],
             modified: ['staged-modified', 'modified', 'staged'],
          }));
       });
 
       it('modified status', () => {
-         const statusSummary = parseStatusSummary(`
-             M package.json
-            M  src/git.js
-            AM src/index.js
-             A src/newfile.js
-            ?? test
-            UU test.js
-      `);
+         const statusSummary = parseStatusSummary(` M package.json${NULL}M  src/git.js${NULL}AM src/index.js${NULL} A src/newfile.js${NULL}?? test${NULL}UU test.js`);
 
          expect(statusSummary).toEqual(like({
             created: ['src/index.js', 'src/newfile.js'],
@@ -356,10 +359,8 @@ R  src/a.txt -> src/c.txt
       });
 
       it('index/wd status', () => {
-         const statusSummary = parseStatusSummary(` M src/git_wd.js
-MM src/git_ind_wd.js
-M  src/git_ind.js
-`);
+         const statusSummary = parseStatusSummary(statusResponse('main',
+            ` M src/git_wd.js`, `MM src/git_ind_wd.js`, `M  src/git_ind.js`).stdOut);
          expect(statusSummary).toEqual(like({
             files: [
                {path: 'src/git_wd.js', index: ' ', working_dir: 'M'},
@@ -370,21 +371,16 @@ M  src/git_ind.js
       });
 
       it('Report conflict when both sides have added the same file', () => {
-         expect(parseStatusSummary(`## master\nAA filename`)).toEqual(like({
+         expect(parseStatusSummary(statusResponse(`master`, `AA filename`).stdOut)).toEqual(like({
             conflicted: ['filename'],
          }));
       });
 
       it('Report all types of merge conflict statuses', () => {
-         const statusSummary = parseStatusSummary(`
-            UU package.json
-            DD src/git.js
-            DU src/index.js
-            UD src/newfile.js
-            AU test.js
-            UA test
-            AA test-foo.js
-      `);
+         const statusSummary = parseStatusSummary(statusResponse('branch',
+            'UU package.json', 'DD src/git.js', 'DU src/index.js',
+            'UD src/newfile.js', 'AU test.js', 'UA test', 'AA test-foo.js'
+         ).stdOut);
 
          expect(statusSummary).toEqual(like({
             conflicted: ['package.json', 'src/git.js', 'src/index.js', 'src/newfile.js', 'test.js', 'test', 'test-foo.js']
