Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

We can't choose spinners on Windows besides the Line Spinner #40

Closed
koriolis opened this issue Apr 6, 2017 · 2 comments
Closed

We can't choose spinners on Windows besides the Line Spinner #40

koriolis opened this issue Apr 6, 2017 · 2 comments

Comments

@koriolis
Copy link

koriolis commented Apr 6, 2017

The following line 22 on the Ora constructor always forces the Line spinner on Windows, even if we choose another spinner:

this.spinner = typeof sp === 'object' ? sp : (process.platform === 'win32' ? cliSpinners.line : (cliSpinners[sp] || cliSpinners.dots)); // eslint-disable-line no-nested-ternary

I accept it the reasoning to force a different default value for windows instead of dots as usually normal unconfigured CMD or PowerShell are not configured to handle most special characters.

Simply blocking the option to select another spinner is a bit excessive in my opinion, and blocks functionality.

As an example here's VSCode Integrated Terminal with Powershell running the Dots spinner (modified the local Ora code directly):

ora_dots_spinner

On the other hand if we changed the code above to something like:

this.spinner = typeof sp === 'object' ? sp : (cliSpinners[sp] || (process.platform === 'win32' ? cliSpinners.line : cliSpinners.dots)); // eslint-disable-line no-nested-ternary

Then by default Windows gets the line spinner, everybody else gets dots, but we can still choose another spinner in the options if desired.

@sindresorhus
Copy link
Owner

Happy to add better detection for when Unicode is fully supported unconfigured, but the current behavior is intentional so consumers of this module doesn't need to care about having platform conditional code just to specify a spinner.

@koriolis
Copy link
Author

koriolis commented Apr 6, 2017

Fair enough.
But in practice this behaviour also forces the consumers of the module that actually want to display the other spinners in all platforms (say to build an Atom or VSCode plugin or extension for example) to having to re-define the spinners themselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants