search

observablehq / framework

A static site generator for data apps, dashboards, reports, and more. Observable Framework combines JavaScript on the front-end for interactive graphics with any language on the back-end for data analysis.
https://observablehq.com/framework/
ISC License
2.13k stars 85 forks source link

if stdout is not a tty, replace clack.spinner by console.log #1473

Closed Fil closed 2 days ago

Fil commented 1 week ago

closes #1447 closes #1446

I'm not happy that I had to make c8 specifically ignore the non-coverage of the non-tty output tests, but I did not find a better way. (solved in the new approach, see below)

mythmon commented 4 days ago

Generally we've been pushing these tty checks higher in the call stack, so that we can have different messages on non-TTYs. In the deploy effects we have a isTty variable that we can use in tests to test both kinds of messages. I'd prefer if we keep that pattern instead of making Clack sometimes behave differently.

Fil commented 4 days ago

isTty tests whether stdin is a tty, which will orient the questions one way or the other.

This by contrast is checking if stdout is a tty, so that we know not to output control characters; this is similar to what we do with color() in https://github.com/observablehq/framework/blob/f0e45f1e4490592dff4aa476f7d553fca77e8b39/src/tty.ts#L21

(In a sense, isTty is poorly named.)

mythmon commented 4 days ago

I suppose we should add isStdinTty and isStdoutTty (I'd keep a isTty that is equal to isStdinTty && isStdoutTty too).

My point was more that we should push this up the stack, and leverage the effect system instead of doing it at import time.

Fil commented 2 days ago

@mythmon I've tried a different approach, hopefully closer to what you had in mind?