forest icon indicating copy to clipboard operation
forest copied to clipboard

Remove home-rolled `prompt_confirm` function

Open aatifsyed opened this issue 2 years ago • 1 comments

  • [x] remove the atty crate in favour of IsTerminal
  • [x] standardize on the indicatif/dialoguer UI crates
    • [ ] remove home-rolled prompt_confirm function
      • Also, as far as I remember, we re-implemented it here for this reason: https://github.com/console-rs/dialoguer/issues/170 But it would be nice not to use such workarounds.

        Originally posted by @LesnyRumcajs in https://github.com/ChainSafe/forest/pull/2893#discussion_r1217710558

    • [x] rationalize our progress bars and color story
  • [ ] ~~log to stderr, not stdout~~ The logs are the primary out of forestd and should be sent to stdout. The other tools (forest-wallet, forest-tool, forest-cli) already log to stderr.

aatifsyed avatar Jun 05 '23 09:06 aatifsyed

It's likely that a lot of our atty checks have subtle/edge bugs. E.g https://github.com/ChainSafe/forest/blob/1af4fde3cb1ef3e24a3ff9d7082d171d4c8b1892/forest/daemon/src/daemon.rs#L518 Checks that stdin is a tty, but the confirm is actually rendered on stdout, and will actually error on its own

aatifsyed avatar Jun 06 '23 21:06 aatifsyed