SBoudrias / Inquirer.js

A collection of common interactive command line user interfaces.
MIT License
20.29k stars 1.3k forks source link

Feat(@inquirer/core): Pass status to `usePrefix`/`theme.message` + Change default prefix #1544

Closed nvlang closed 2 months ago

nvlang commented 2 months ago

This PR includes 5 commits:

What's missing is a unit test for the changes to style.message. However, I couldn't immediately find unit tests for style.message to use as a template, and I didn't want to write one from scratch, since I'm not familiar with the setup. I tested the new style.message functionality locally via corepack yarn demo, and there it seemed to work fine.

Closes #1537.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Please upload report for BASE (main@d2690cc). Learn more about missing BASE report.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1544 +/- ## ======================================= Coverage ? 97.78% ======================================= Files ? 39 Lines ? 2391 Branches ? 644 ======================================= Hits ? 2338 Misses ? 46 Partials ? 7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nvlang commented 2 months ago

I think ? for when the question is pending and for when it's done could make sense? And maybe making the question mark blue and the checkmark green? I had left it alone since it seemed like it'd be a matter of personal preference, but I'd be glad to try to add whatever default you'd prefer.

Also, I wholeheartedly agree about giving status specific allowed strings — if I recall correctly, I had seen 'pending', 'loading', and 'done' being used, but also 'idle' in a unit test somewhere. I assume the first three should suffice, or would it make sense to include more statuses in your opinion?

SBoudrias commented 2 months ago
  1. Prefix, yeah blue/green question mark/check mark sounds good to me.

  2. I think we can go with 3 status. I prefer idle to pending; so I'd normalize this way. (maybe you'll find a valid 4th one when running typecheck, so low risk of missing one.)

nvlang commented 2 months ago

@SBoudrias I guess changing the default prefix means that we should also update the screenshots in the READMEs — however, I'm not sure what the setup / workflow there is (theme, font, etc.).

SBoudrias commented 2 months ago

Thanks for the work you've put on that PR!

I went ahead a made a few adjustments before merging (just an FYI):

SBoudrias commented 2 months ago

About the screenshots, they were first introduced in #625... Feel free to regenerate screenshot and document a reproducible process in the CONTRIBUTING.md doc! (not sure if the same will work out of the box today 😬 - it's from 2018)