Commit 5323178

Anton Golub <antongolub@antongolub.com>
2024-05-27 18:28:26
fix: apply EOL ensurer to the last chunk only (#825)
Co-authored-by: Anton Medvedev <anton@medv.io>
1 parent 558be70
Changed files (3)
src/core.ts
@@ -18,6 +18,7 @@ import { type Encoding } from 'node:crypto'
 import { AsyncHook, AsyncLocalStorage, createHook } from 'node:async_hooks'
 import { Readable, Writable } from 'node:stream'
 import { inspect } from 'node:util'
+import { EOL } from 'node:os'
 import {
   exec,
   buildCmd,
@@ -39,7 +40,6 @@ import {
   quote,
   quotePowerShell,
   noquote,
-  ensureEol,
   preferNmBin,
 } from './util.js'
 
@@ -54,6 +54,7 @@ export interface Shell {
 
 const processCwd = Symbol('processCwd')
 const syncExec = Symbol('syncExec')
+const eol = Buffer.from(EOL)
 
 export interface Options {
   [processCwd]: string
@@ -296,9 +297,13 @@ export class ProcessPromise extends Promise<ProcessOutput> {
           // Stderr should be printed regardless of piping.
           $.log({ kind: 'stderr', data, verbose: !self.isQuiet() })
         },
-        end: ({ error, stdout, stderr, stdall, status, signal }) => {
+        end: ({ error, stdout, stderr, stdall, status, signal }, c) => {
           self._resolved = true
 
+          // Ensures EOL
+          if (stderr && !stderr.endsWith('\n')) c.on.stderr?.(eol, c)
+          if (stdout && !stdout.endsWith('\n')) c.on.stdout?.(eol, c)
+
           if (error) {
             const message = ProcessOutput.getErrorMessage(error, self._from)
             // Should we enable this?
@@ -426,7 +431,7 @@ export class ProcessPromise extends Promise<ProcessOutput> {
   }
 
   pipe(dest: Writable | ProcessPromise): ProcessPromise {
-    if (typeof dest == 'string')
+    if (typeof dest === 'string')
       throw new Error('The pipe() method does not take strings. Forgot $?')
     if (this._resolved) {
       if (dest instanceof ProcessPromise) dest.stdin.end() // In case of piped stdin, we may want to close stdin of dest as well.
@@ -720,7 +725,7 @@ export function log(entry: LogEntry) {
     case 'stdout':
     case 'stderr':
       if (!entry.verbose) return
-      process.stderr.write(ensureEol(entry.data))
+      process.stderr.write(entry.data)
       break
     case 'cd':
       if (!$.verbose) return
src/util.ts
@@ -433,11 +433,3 @@ export function getCallerLocationFromString(stackString = 'unknown') {
       ?.trim() || stackString
   )
 }
-
-export function ensureEol(buffer: Buffer): Buffer {
-  if (buffer.toString('utf8', buffer.length - 1) !== '\n') {
-    return Buffer.concat([buffer, Buffer.from('\n')])
-  }
-
-  return buffer
-}
test/util.test.js
@@ -29,7 +29,6 @@ import {
   getCallerLocationFromString,
   tempdir,
   tempfile,
-  ensureEol,
   preferNmBin,
 } from '../build/util.js'
 
@@ -168,23 +167,6 @@ test('tempfile() creates temporary files', () => {
   assert.equal(fs.readFileSync(tf, 'utf-8'), 'bar')
 })
 
-test('ensureEol() should ensure buffer ends with a newline character', () => {
-  // Test case 1: Buffer without newline
-  const buffer1 = Buffer.from('Hello, world!')
-  const result1 = ensureEol(buffer1).toString()
-  assert.strictEqual(result1, 'Hello, world!\n')
-
-  // Test case 2: Buffer with newline
-  const buffer2 = Buffer.from('Hello, world!\n')
-  const result2 = ensureEol(buffer2).toString()
-  assert.strictEqual(result2, 'Hello, world!\n')
-
-  // Test case 3: Empty buffer
-  const buffer3 = Buffer.from('')
-  const result3 = ensureEol(buffer3).toString()
-  assert.strictEqual(result3, '\n')
-})
-
 test('preferNmBin()', () => {
   const env = {
     PATH: '/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/sbin',