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

Improvement performance of the .clear() method #171

Closed
wants to merge 2 commits into from

Conversation

moofoo
Copy link
Contributor

@moofoo moofoo commented Feb 19, 2021

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 this issue.

Tests can be found here

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
Copy link
Owner

Tests can be found here

Could you include the tests in this PR?

@sindresorhus sindresorhus changed the title Improvements to clear method Improvement performance of the .clear() method Feb 20, 2021
@promaty
Copy link

promaty commented May 11, 2021

Will this fix WSL flickering issue?

@jimmmeh
Copy link

jimmmeh commented Jun 26, 2021

Is there any reason this hasn't been merged, @sindresorhus? It makes a huge difference on the windows systems I have tried and seemingly all tests have passed?

@G-Rath
Copy link
Contributor

G-Rath commented Aug 15, 2021

I applied this locally and found it completely eliminates the minor flicker I'd observed when developing my cli app, so would love to help get this landed.

@sindresorhus I've got an updated version of this branch with the tests from the linked commit cherry-picked in, with everything passing - is there more work you'd like done before landing this, or should I go ahead with creating a PR?

@sindresorhus
Copy link
Owner

@G-Rath Go ahead with the PR.

@G-Rath
Copy link
Contributor

G-Rath commented Aug 15, 2021

Have created #182

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

5 participants