feat: add taskLog prompt

[43081j]

Adds a new taskLog prompt which renders messages then ultimately does one of the following:

• If error, retain the output and render the error message
• If success, clear the output and render the success message

For example:

const log = prompts.taskLog({
  message: 'some message'
});

log.message('a line of text');
log.message('another line of text');
log.message('multiple\nlines\nof\ntext');

// at this point, it will have rendered like so:
// o some message
// | a line of text
// | another line of text
// ...

log.error('some error');

// if the line above runs, it will render like so:
// x some error
// | a line of text
// | another line of text
// ...

log.success('some success');

// if the line above runs, it will clear and render like so:
// o some success

part of bringing svelte CLI back to using clack instead of a fork

[changeset-bot]

🦋 Changeset detected

Latest commit: e41b36b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@clack/prompts	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

@example/basic • @example/changesets

npm i https://pkg.pr.new/bombshell-dev/clack/@clack/core@276

npm i https://pkg.pr.new/bombshell-dev/clack/@clack/prompts@276

commit: e41b36b

[manuel3108]

Overall this looks perfect, although I'm having trouble integrating this into the svelte cli sv. But maybe it's just too late for me.

This is how it's currently implemented:
https://github.com/sveltejs/cli/blob/ccb512d1bb48992dcff9cacadd063085789786fd/packages/cli/utils/package-manager.ts#L43-L69

Currently the following exception is printed:

TypeError: a.split is not a function

I'm assuming this is related to this pr, but from the code i don't understand exactly where that is happening.

[manuel3108]

Suggested change

	for await (const line of npmInstall()) {
	for await (const line of npmInstall()) {

Not sure if the npmInstall makes it clear enough for the people, but I don't have any better idea

[43081j]

Ah I may have to tweak it a little

I feel like I remember chunks from a stream not always being line based for stdout. So we probably need to join them up as a string and split again, rather than assuming each one is a line

I'll take a look. That doesn't fix your problem but I noticed it when reading the usage you linked

[manuel3108]

Actually, I think that could exactly be the problem I'm experiencing.

[43081j]

We already export cancel actually too. I forgot we had that, so we don't need to change it 👍

[43081j]

@manuel3108 (and maybe @natemoo-re review):

I've added a raw option:

log.message('line 0', {raw: true});
log.message('still line 0', {raw: true});
log.message('\nline 1', {raw: true}); // now we render another line because we have an explicit `\n`

when it is false, every message call is treated as an individual line of text

[manuel3108]

Sound's logical, but sadly I'm still getting the same error. Here is a repro: https://stackblitz.com/edit/node-edenh16x?file=index.js

[43081j]

that is most likely because the event handlers of stdout can receive a Buffer

if you call toString() on the data first, it should work

though the clearing seems a little buggy since some lines don't get cleared which i'd expect to have been

[43081j]

@manuel3108 i fixed the bug which was causing leftover buffer lines

your stackblitz works now if you toString the chunks

[AdrianGonz97]

thanks for your work on this!!

aside from the suggested changes below, it would also be nice if the rendered logs were dimmed with colors.dim(..) so that they'd have some nice contrast from the title

e.g. in sv, normal and error logs would appear as so:

[43081j]

aside from the suggested changes below, it would also be nice if the rendered logs were dimmed with colors.dim(..) so that they'd have some nice contrast from the title

this is a bug, it was meant to be dim already. so i'll fix that when i get chance 👍

[43081j]

two updates:

• text is now dim
• there's a spacing option to control the spacing between the title and the output

[43081j]

ok @AdrianGonz97 @manuel3108 here's where i ended up:

• a new retainLog option to the taskLog prompt
  • if this is true, the entire log is retained in case you render the log on completion
• a new renderLog option to the success/error methods
  • if this is true, the log is rendered after the success/error message
  • by default, success will set this to false
  • by default, error will set this to true
  • if retainLog was true or limit is not set, this will output the full log. otherwise, it will output the limited log

thoughts?

i am still wondering if retainLog is actually something like ignoreLimitOnComplete 😅 names are tough

[AdrianGonz97]

@43081j
i love this!!

i am still wondering if retainLog is actually something like ignoreLimitOnComplete 😅 names are tough

i think retainLog is perfect

---

the only issue i'm seeing now is that the text dimming is only partially working.

here it is on failures:

it similarly fails during the normal process as well:

this seemingly has to do with how the buffer is derived with the limit. im guessing the starting part of the dim's ansi escape code is being chopped off (see the code review)

here's a small repro:

import { setTimeout } from 'node:timers/promises';
import * as p from '@clack/prompts';

p.intro('some-app');

const t = p.taskLog({ title: 'some title', retainLog: true, limit: 3 });

t.message('foo\nbar\nbaz\nfoobar');
await setTimeout(2000); // small timeout to view it during the regular process
t.error('error!');

p.cancel('ahhh');

[AdrianGonz97]

log.message ended up being the culprit of the partial coloring. more specifically, the coloring of the symbols of the gutter (│).

as the symbols contained the same ansi escape codes as the message's content (e.g. \x1B[2m│\x1B[22m \x1B[2msome message goes here\x1B[22m), the terminating end of the ansi code was not registering properly in multi-line messages.

for example:

\x1B[2m│\x1B[22m  \x1B[2mfoo
\x1B[2m│\x1B[22m  bar
\x1B[2m│\x1B[22m  baz
\x1B[2m│\x1B[22m  \x1B[22m

it thinks the sequence terminates on the \x1B[22m in the second line. as a result, only the first line of the message is properly colored.

the solution was to have each line wrapped with the proper code:

\x1B[2m│\x1B[22m  \x1B[2mfoo\x1B[22m
\x1B[2m│\x1B[22m  \x1B[2mbar\x1B[22m
\x1B[2m│\x1B[22m  \x1B[2mbaz\x1B[22m

[43081j]

its basically because in raw mode, a message can span multiple lines

so what you're seeing is

<X>|</X> <DIM>message 1
<X>|</X>. still message 1</DIM>

from 'message 1\nstill message 1'

[43081j]

i had to modify what you had a little since we need the spacing to be different

but this should all be good now.

i've also renamed the renderLog option to showLog, which feels a bit nicer

[43081j]

@AdrianGonz97 @manuel3108 hows things looking on your end?

if all is good, im happy to merge this once i get an approval

[AdrianGonz97]

For the taskLog, this is now working perfectly, thank you!!

The only thing left that we're missing is a replacement for our box, which is effectively a note but without the dimming. Not sure if it's solvable in userland, but I'll give it a try in a few.

[43081j]

you could probably just msg.split('\n').map(color.dim).join('\n') essentially, and pass that into note

but maybe it is worth us having some kind of option for this 🤔 though im not sure dimmed makes sense as maybe you want it any colour, or any format. not sure where we'd draw the line there, e.g. is it just a format: (line) => string option

[AdrianGonz97]

you could probably just msg.split('\n').map(color.dim).join('\n') essentially, and pass that into note

tried p.note(msg.split('\n').map(color.reset).join('\n'), "some title"); but ended up with some weirdness where the last line gets partially dimmed 😅

not sure where we'd draw the line there, e.g. is it just a format: (line) => string option

a format option would be lovely!

[43081j]

no worries i think this is just because we're hackily putting it into a dim(msg) overall

ill open a separate PR to make that configurable some way

[manuel3108]

Just tried this out in the draft PR for sv, and this looks perfect. Thank you so much for your help!

I think the new format option for note should be the final missing piece to get exactly the same functionality as we have right now. But as you said, that can happen separately.

[43081j]

I already opened #284 for that FYI

@natemoo-re could I get your review at some point? Once this is approved I will merge

[dreyfus92]

this is pretty solid james, LGTM 🫡

[natemoo-re]

Looks awesome! My only comment is on naming—does taskLog effectively communicate what this does? Has something like subtask(), subprocess(), or just pipe() been considered?

Not going to block this on an imperfect name, though. Code looks great! Whatever you decide is great 😊

[43081j]

its not necessarily from a sub-process, keep in mind

we may use it just for a windowed log from some arbitrary stream

[43081j]

i pushed an update FYI

i noticed in CI each limit window gets logged since its just a text log and isn't a TTY (so can't erase/move)

so i've updated message to render nothing in CI. this way, when you call error or success, we will render the whole log (assuming you enabled that option).