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

Update the string that clears the console. #2071

Merged
merged 2 commits into from
Jan 9, 2018

Conversation

DanielBadan
Copy link
Contributor

#1914
I've tested it with Windows 10 and 7, node versions from ~5.0.0 up to 7.7.0.
Didn't managed to test it on 8 but it should work to.

Before

After
2017-05-03_1204

facebook#1914
I've tested it with Windows 10 and 7, node versions from ~5.0.0 up to 7.7.0.
Didn't managed to test it on 8 but it should be fine.
@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions.

@gaearon
Copy link
Contributor

gaearon commented May 4, 2017

I'm pretty sure \x1Bc was added intentionally on Windows. I don’t remember why though 😞

@DanielBadan
Copy link
Contributor Author

It's probably not relevant any more. It worked for me without \x1Bc but I guess would be useful to have at least one more person to test that.

@Timer
Copy link
Contributor

Timer commented May 8, 2017

In the beginning we only used \x1bc, and then eventually switched to an alternative sequence for macOS because you could do fancier things.

Eventually we ended up copying Jest via jestjs/jest#2230.

Jest's latest sequence is https://github.com/facebook/jest/blob/master/packages/jest-cli/src/constants.js#L15, which appears identical to what we're still doing.

We can try \033c or \033[2J\033[0f (for windows).

Add windows specific string for clearing the console.
@DanielBadan
Copy link
Contributor Author

It seems to work with \033[2J\033[0f, only changed the numbers to Hex because of use strict so it's \x1B[2J\x1B[0f.

In windows 10 it clears the console with Git Bash, CMD and Powershell.
For 7 & 8 it clears the console with CMD and Powershell, no Git Bash.

@gaearon
Copy link
Contributor

gaearon commented May 17, 2017

Am I correct that the current string fails in GitBash but works in other environments?
And that the proposed string fixes GitBash on Windows 10 but has no effect in other envs?

@DanielBadan
Copy link
Contributor Author

Yes.
Windows 10: CMD, Powershell, Git Bash.
Windows 8: CMD, Powershell.
Windows 7: CMD, Powershell.

@gaearon
Copy link
Contributor

gaearon commented May 18, 2017

I’ll test this later today.

@gaearon gaearon added this to the 1.0.1 milestone May 18, 2017
@gaearon
Copy link
Contributor

gaearon commented May 18, 2017

We’ll need a matching PR to Jest.

@gaearon gaearon modified the milestones: 1.0.1, 1.0.x May 19, 2017
@gaearon gaearon modified the milestones: 1.x, 1.0.x Jun 27, 2017
@gaearon gaearon mentioned this pull request Jun 27, 2017
@gaearon gaearon modified the milestones: 1.0.x, 1.x Jun 27, 2017
@gaearon
Copy link
Contributor

gaearon commented Jun 28, 2017

This does seem to work better on a new Windows laptop I'm using. I don't understand why though. It even works in PowerShell but old sequence doesn't (with PowerShell too) on my new Windows laptop. How is this possible? Could some new Windows 10 versions break the old sequence?

@DanielBadan
Copy link
Contributor Author

Probably just another weird thing of windows, the original sequence stopped working for me after an update.

@gaearon
Copy link
Contributor

gaearon commented Jan 8, 2018

Can you send this PR to Jest too? https://github.com/facebook/jest

@gaearon
Copy link
Contributor

gaearon commented Jan 8, 2018

Never mind, I sent jestjs/jest#5251.

@gaearon gaearon merged commit d3a1765 into facebook:master Jan 9, 2018
@gaearon gaearon modified the milestones: 1.0.x, 1.0.18 Jan 9, 2018
@gaearon gaearon mentioned this pull request Jan 15, 2018
@gaearon
Copy link
Contributor

gaearon commented Jan 15, 2018

This is out in [email protected]! Please let us know if something doesn’t quite work.
https://github.com/facebookincubator/create-react-app/releases/tag/v1.1.0

Pavek pushed a commit to Pavek/create-react-app that referenced this pull request Jul 10, 2018
* Update the string that clears the console.

facebook#1914
I've tested it with Windows 10 and 7, node versions from ~5.0.0 up to 7.7.0.
Didn't managed to test it on 8 but it should be fine.

* Update windows string

Add windows specific string for clearing the console.
@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants