search

jaredpalmer / tsdx

Zero-config CLI for TypeScript package development
https://tsdx.io
MIT License
11.24k stars 508 forks source link

Add option to not show progress animations in logs #942

Open garrettjstevens opened 3 years ago

garrettjstevens commented 3 years ago

Fixes https://github.com/formium/tsdx/issues/689.

Adds a --noProgress flag (based on lerna's --no-progress, but made camelCase to match existing flags). When used, the progress-estimator logger is replaced by a simple console.log of the message. The progress animations will also be removed when the environment is not a TTY and when running in CI.

Thanks for a great tool, and let me know if there's anything I can do to improve this PR.

vercel[bot] commented 3 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/formium/tsdx/dxo4hh6o4
✅ Preview: https://tsdx-git-fork-garrettjstevens-noprogressanimation.formium.vercel.app

garrettjstevens commented 3 years ago

Thanks @agilgur5, I'll work on addressing those issues.

garrettjstevens commented 3 years ago

A few comments on my last commit:

  • The comment in SharedOptions is not correct (changing noProgress in the config does not change it, so the word "Should" is confusing and potentially misleading; "Is output progress being shown?" would be better)

Since the option is a negative, I went with "Are output progress animations disabled?". Is that ok?

  • Only 1 command was updated (build and watch should both be updated)

I've added the option to watch as well. I did this by using the isEnabled option of the ora spinner.

  • The isTTY portion is a breaking change and therefore can only go out in a breaking release. I would recommend that part be separated into a different PR as such.

It's now removed from this PR. I'll open a draft PR with that change separately.

  • Is there a way to infer Lerna's current progress output status? That way there would not necessarily be a need for a new flag, which is what I was hoping to avoid in the issue

In my use case I'm not using Lerna, so I'm hoping we can add this new flag.