Add --group option

[cdrini]

Closes #75. This option makes it so that the output is displayed as if the commands were run sequentially.

The current approach adds an intercepting layer between Logger and the actual output stream called OutputWriter, which buffers each command's output as necessary. This worked nicely, because it was completely orthogonal to any of the other code, but did result in a few breaking changes to the APIs:

• Logger no longer takes in an outputStream; it has an observable which it emits to. This makes Logger essentially a logger formatting class.
• Logger#log and friends now pass along the command value, so that OutputWriter can use it for buffering.
• concurrently now takes in logger and outputStream and wires them together using the new OutputWriter class.

Examples:

# Util script to print a number every $1 seconds
echo -e '#!/bin/bash\n\nfor i in {1..10}; do sleep $1 && echo $2$i; done' > print-interval.sh

# A few variations
node bin/concurrently.js -p '[{index}] [{time}]' --group 'bash print-interval.sh 0.5 A' 'bash print-interval.sh 1 B' 'bash print-interval.sh 1.5 C'
node bin/concurrently.js -p '[{index}] [{time}]' --group 'bash print-interval.sh 1.5 A' 'bash print-interval.sh 1 B' 'bash print-interval.sh 0.5 C'
node bin/concurrently.js -p '[{index}] [{time}]' --group 'bash print-interval.sh 0.5 A' 'bash print-interval.sh 1.5 B' 'bash print-interval.sh 1 C'

# Some more random timings
node bin/concurrently.js -p '[{index}] [{time}]' --group 'npx jest --coverage=false src/command.spec.js' 'npx jest --coverage=false src/completion-listener.spec.js' 'npx jest --coverage=false src/concurrently.spec.js' 'npx jest --coverage=false src/get-spawn-opts.spec.js' 'npx jest --coverage=false src/logger.spec.js' 'npx jest --coverage=false src/output-writer.spec.js'

[coveralls]

Coverage decreased (-0.7%) to 99.31% when pulling 6aac78b on cdrini:feature/group-option into ccb6b8d on kimmobrunfeldt:master.

[gustavohenke]

Hey! Thanks for this PR, it looks like a really simple approach!

One major thing, though: I think that we should flush buffers as soon as commands close.

./bin/concurrently.js --group "sleep 3; echo hi; sleep 3; echo hey" "echo foo"
[0] hi
[0] hey
[0] sleep 3; echo hi; sleep 3; echo hey exited with code 0
[1] foo
[1] echo foo exited with code 0

In this example, echo foo finished instantly, however the buffer is not flushed, leading to increased memory usage.

Also, it just feels like a poorer experience that you don't get to know that your command finished early. In the example above, that was only 6 seconds, but it could be much worse, of course.

WDYT?

---

And can you please remove the TypeScript annotations?
None of the files have this but the ones you touched now :/

[gustavohenke]

For a nicer API... what do you think if Logger had a subscribe method?

Suggested change

	options.logger.observable.subscribe(({command, text}) => outputWriter.write(command, text));
	options.logger.subscribe(({command, text}) => outputWriter.write(command, text));

[cdrini]

I initially had this actually! I ended up removing it because then it made look logger look like an Observable, which I thought might be confusing (cause then folks might expect it to have other Observable methods/props). Then I thought maybe I should make Logger extends Observable, and then I decided I'd just KISS and use composition :P

[Snaptags]

@cdrini I really like your --group option. Could you perhaps evaluate the change requests? I'd love to see the feature in a release.

[cdrini]

Goodness has it really been four months 😅 Sorry for the delay folks! Taking a look at this today 👍 Thanks for the review @gustavohenke ! And thanks for the +1 @Snaptags :)

[cdrini]

Ok! Rebased off latest master, and confirmed working 👍

1. Flushing output as soon as command exits

   • So my main use case for this feature is that in https://github.com/internetarchive/bookreader/blob/master/package.json, our build step looks something like this: npm run clean && npm run build-js && npm run build-css && npm run build-assets. I'd love to be able to run the js/css/asset building in parallel, but the output becomes garbled and illegible. So for me, I specifically want the output to appear as if they ran in sequence, even if one finishes earlier than the others. (Also the tests: npm run test-jest && npm run test-karma). If I'm debugging performance, then adding something like -p '[{index}] [{time}]' shows the actual time things ran/exited.

2. TS/JSDOC

   • Ah, are you sure? 😅 One of the hardest parts for me jumping into this code was trying to figure out what exactly a Command was, since a few different things are referred to by that name. Would love to save the next person some time. But I understand the importance of consistency, so I will remove them if you think that's best 👍

[gustavohenke]

So for me, I specifically want the output to appear as if they ran in sequence, even if one finishes earlier than the others.

We can tackle that independently, e.g. #257 asks for the sequential running.

One of the hardest parts for me jumping into this code was trying to figure out what exactly a Command was, since a few different things are referred to by that name. Would love to save the next person some time.

Understandable!
I love TS, but the annotations are not fit for the codebase yet.

Some of the dependencies have started using ES Modules, and I'd hate to force concurrently users into ESM.
So one of the alternatives I've been thinking is to use Typescript. Maybe that would provide an even better developer experience for folks using the programmatic API.

[coveralls]

Coverage decreased (-0.7%) to 99.02% when pulling 7b77967 on cdrini:feature/group-option into ba384de on open-cli-tools:master.

[cdrini]

No worries, I removed the type hints :) I find the comment typehints to be a really great way to get 95% of the benefits of type annotations, without having to require users to do anything special. With comments, you don't have to switch the entire codebase to ESM or TS, but you do get that sweet sweet auto-complete in vs code, which is why I love it! :) Eg

---

Oh, #257 is different; that is about actually running the commands in sequence. I believe the main purpose of the --group feature request is to make the output look like it ran sequentially, while still getting the performance benefits of parallelism.

EDIT: I think this is what other folks are asking for as well: #75 (comment) . Flushing output on exit would cause the output to be garbled/inter-mixed.

[cdrini]

@gustavohenke Anything I can do to help move this along? I'd love to be able to start using this :)

[gustavohenke]

redacted

[gustavohenke]

Overall idea and implementation here LGTM!

I've started porting the codebase to TypeScript, so this has obviously caused a bunch of conflicts 😅
But since it was me who held this PR open for so long, I'll fix those.

Furthermore, I'm thinking about a different name for the feature, and maybe refactoring some bits, so I'll push that to your branch as well.

[gustavohenke]

Happy new year! This is now out in v7.0.0.

[cdrini]

Well isn't that the sweetest New Year's present! Awesome! Thank you so much! And absolutely no worries ; I know you're busy, and I know I haven't exactly been super on top of it either 😅 Hope the TS migration wasn't too difficult!

Happy New Year! 🥂