microlinkhq / tinyrun

CLI for executing multiple commands in parallel with minimal footprint (~2KB).
MIT License
6 stars 1 forks source link

Incorrect line prefixing #3

Closed aleclarson closed 1 month ago

aleclarson commented 1 month ago

The way you're prefixing assumes that stdout/stderr "data" events always emit entire lines, which can be demonstrated to be false by using the @biomejs/biome npm package's biome check command.

Example output:

     biome The number of diagnostics exceeds the number allowed by Biome.
     biome Diagnostics not shown: 230.
     biome Checked 179 files in 249ms. No fixes applied.
     biome Found 249 errors.
     biome chec     biome k ━━━━━━━━━━━━━━━━━━━━     biome ━━━━━━━━━━━━━━━     biome ━━━━━━━━━━━━     biome ━━━━━━━━━━━━━━━━━━     biome ━━━━━━━━━━━━━     biome ━━━     biome ━━━━━━━━━━━━━

     biome   × Some erro     biome rs were emitted while running checks.
aleclarson commented 1 month ago

Here's the patch I'm using to fix this:

diff --git a/bin/index.js b/bin/index.js
index 42085ea0afcd5485dadc9573eb4ea39c2a6b8dab..888a4e436d7a0bddfae21b5fa361e8c2d469f415 100755
--- a/bin/index.js
+++ b/bin/index.js
@@ -18,17 +18,13 @@ const prettyMs = ms => {
   return `${minutes}m ${remainingSeconds.toFixed(2)}s`
 }

-const toStream =
-  stream =>
-    (data, { name }) =>
-      stream.write(split(data, name))
-
-const split = (text, prefix) =>
-  text
-    .toString()
-    .split('\n')
-    .map(line => (line ? `${prefix} ${line}` : ''))
-    .join('\n')
+const toStream = stream => (data, task) => {
+  const lines = (task.buffer += data.toString()).split('\n')
+  if (data.length) task.buffer = lines.pop() || ''
+  for (const line of lines) {
+    stream.write(task.name + (line ? ` ${line}` : '') + '\n')
+  }
+}

 const { _, ...flags } = require('mri')(process.argv.slice(2))

@@ -46,7 +42,8 @@ if (Array.isArray(names) && names.length > 0) {

 const tasks = _.map((cmd, index) => ({
   cmd,
-  name: styleText(getColor(index), names?.[index] ?? `[${index}]`)
+  name: styleText(getColor(index), names?.[index] ?? `[${index}]`),
+  buffer: ''
 }))

 const stdout = toStream(process.stdout)
diff --git a/src/index.js b/src/index.js
index 4d8fff4e6c24926139316b615ac50e6d92394973..f638c92076ccfb1a93f6d223a355746b4ccfae87 100644
--- a/src/index.js
+++ b/src/index.js
@@ -17,7 +17,9 @@ module.exports = ({ tasks, childOpts, start, exit, ...pipes }) =>
       const duration = timeSpan()

       ;['stdout', 'stderr'].forEach(pipe =>
-        subprocess[pipe].on('data', data => pipes[pipe](data, task))
+        subprocess[pipe]
+          .on('data', data => pipes[pipe](data, task))
+          .on('end', () => pipes[pipe]('', task))
       )

       subprocess.once('exit', async exitCode => {
Kikobeats commented 1 month ago

@aleclarson Thanks! I turned your diff into a PR 🙂

The only thing I miss is an unit test to verify this change is necessary.

Can you suggest me a way to get the a similar output than biome check?

aleclarson commented 1 month ago

5

Kikobeats commented 1 month ago

Great! published tinyrun@1.0.1

How did you discover this dependency? 🙂

aleclarson commented 1 month ago

How did you discover this dependency? 🙂

First page of https://www.npmjs.com/search?q=concurrently 😄