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

Improve performance of the .clear() method #182

Merged
merged 3 commits into from Aug 23, 2021
Merged

Improve performance of the .clear() method #182

merged 3 commits into from Aug 23, 2021

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Aug 15, 2021

Rebasing of #171 with tests cherry picked across

Closes #171
Closes #170

moofoo and others added 3 commits August 15, 2021 13:31
changes:
- including spinner.indent in the calculation of lineCount

- calling updateLineCount in indent setter
- cursorTo is called the minimal number of times necessary: to set the cursor position to the start of the line to set up the clearLine(1) loop, and after the loop to reset the cursor (if necessary)

- clearLine(0) and clearLine(-1) cause flickering issues in Windows terminals when executed on an interval. Using clearLine(1) resolves that issue.
@sindresorhus sindresorhus merged commit d51c971 into sindresorhus:main Aug 23, 2021
@sindresorhus
Copy link
Owner

Thanks :)

@G-Rath G-Rath deleted the improved-clearing branch August 23, 2021 19:21
@G-Rath
Copy link
Contributor Author

G-Rath commented Aug 23, 2021

@sindresorhus I figure the answer will be "No" but have to ask: is there anyway of getting a v5/commonjs release of this? I like that you're pushing the industry towards faster adoption of ESM and think that's a good thing, but currently I'm not able to do it myself yet due to being very time-limited (+ jest not having good enough support for ESM yet, which feeds into time because I can't consider moving to other libraries right now as it's what we use at work).

I wouldn't expect to have any further versions on v5 afterwards, but this one is something of a game-changer for our Windows users since currently it's a very flashy experience and I've not been able to find any strong alternatives that don't either have this same flaw or others.

I'd be happy to do as much of the work as possible to make it as easy for you as possible :)

@sindresorhus
Copy link
Owner

I don't have any plans to backport. It's not only because of ESM, but also because every change to the rendering risks breaking someone's use-case and I prefer to only have this change in a major release. This is also not a a critical change. Windows users have been seeing flickering for years and have not done anything about it until now. Windows users in general are also used to a worse experience (slower terminal, less compatible with tools, missing Unicode support, etc).

nbl7 pushed a commit to nbl7/ora that referenced this pull request Mar 15, 2022
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

Successfully merging this pull request may close these issues.

None yet

3 participants