Fix progress bar in windows

[KayLeung]

Summary
Fixes #2530. This patch replaces the 2-byte progress bar characters with - and # wrapped in a pair of [ and ] symbols to make it looks like a progress bar on the console with "simple", one-byte characters.

The reason for preferring one-byte characters is the inconsistent width calculation on certain terminal emulators causing the calculated progress bar width to overflow the available terminal width, causing the progress bar to split into multiple lines.

It now looks like this:

Test plan

Manual verification and updating of existing test snapshots.

[BYK]

I don't think this is a fix. this.chars.length will always be 2 so this is essentially dividing the length by two.

I wonder if Consolas is causing double width for these characters for some reason?

[BYK]

Okay, the reason is Consolas not supporting our bar characters properly I think because they are 2-byte unicode characters.

[KayLeung]

@BYK, You are right actually. I found this issue that tells us more info: microsoft/WSL#1907

In short, we don't know the char width.

So in Windows, make the bar half would help.

[BYK]

So in Windows, make the bar half would help.

That would make the experience worse for people like me so I'm not a fan of this idea. May be we can introduce a way to override these characters and default to - and # on Windows? Then I can override them with the originals?

Also, I'm curious if simply setting the stdout encoding to utf-8 would fix it or not.

[KayLeung]

Also, I'm curious if simply setting the stdout encoding to utf-8 would fix it or not.

Nope. It still affected by font in all of my testing.

May be we can introduce a way to override these characters and default to - and # on Windows?

Hmm, not a bad idea. May be we could introduce a way to override this.stdout.columns also?

[BYK]

Nope. It still affected by font in all of my testing.

😿

Hmm, not a bad idea. May be we could introduce a way to override this.stdout.columns also?

Why would that be useful? For this issue, it is not that columns being reported incorrectly but the progress bar characters taking up more space than they should be, right?

[KayLeung]

"fastidious", haha! I like the color bar.

After some thoughts, I think it should be optional for "ambiguous width characters" cause it affected by fonts. And default to 2 in Windows., other platforms would be 1.

[BYK]

And default to 2 in Windows., other platforms would be 1.

May be we can tie this to emoji support detection?

[KayLeung]

@BYK, I do not fully understand your mean. And I think a separate arg will be better.

Here's one of report that it will also happen in MacOS: #2530 (comment)

During researching, I found that this is a pretty common problem: Vim / MacOS Terminal / iTerm also provide this option:
http://vimdoc.sourceforge.net/htmldoc/options.html#'ambiwidth'

I could send a PR if you think it's the right direction. Thanks!

[BYK]

Let's just mimic what Jest does here: https://github.com/facebook/jest/pull/3626/files

Which is using the same block character with different color codes.

[KayLeung]

Sorry @BYK, I linked a bad ref in another issue. I tried it and no helps. I also performed some tests to see if Jest does any other fixes and apparently they have the same problem. But Jest (and NPM) does a simple trick and use a shorter length instead.

https://github.com/npm/npmlog/blob/master/log.js#L45
https://github.com/facebook/jest/blob/master/packages/jest-cli/src/reporters/utils.js#L24

[BYK]

Ah, so the answer lies in either limiting the width or simply not using unicode characters. I think limiting the length is not the safest bet since terminal widths can go even lower and then you have the same issue again. I think we can just do what apt does: use - for "blank" parts and # for the filled parts and put these between [ and ] to form the progress bar appearance. It won't look as cool but it would work everywhere, all the time.

Something like:

[###------] 10%

What do you think?

[KayLeung]

Simple Answer: Yes.

Yeah, it's not the best for appearance but probably the simplest workaround without extra hacks and configs for all platforms users.

[BYK]

Looks like you need to run yarn jest -u to update snapshots (and commit those changes).

This new one looks pretty okay to me. @arcanis @bestander what do you guys think?

[BYK]

Also calling @kaylieEB and @kittens

[bestander]

look fine to me, no onbjections

[BYK]

Let's go!

[swrobel]

EDIT: It's only an issue when my Profile has Advanced > Declare Terminal As set to "rxvt." Can't recall why I set it to that but changing it to "xterm-256color" resolves this.

Unfortunately this is still an issue for me in Terminal.app using the Hack font at various sizes. Here's how it looks:

yarn install v1.0.2-20170914.2147
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...

[-------------------------------------------------------------------------------------------------------------------------------------------------------------------------] 0/911[#########---------------------------------------------------------------------------------------------------------------------------------------------------------------] 50/911[##########################---------------------------------------------------------------------------------------------------------------------------------------------] 141/911[#######################################--------------------------------------------------------------------------------------------------------------------------------] 215/911[##############################################-------------------------------------------------------------------------------------------------------------------------] 252/911[#######################################################----------------------------------------------------------------------------------------------------------------] 301/911[###################################################################----------------------------------------------------------------------------------------------------] 363/911[################################################################################---------------------------------------------------------------------------------------] 439/911[################################################################################################-----------------------------------------------------------------------] 525/911[##############################################################################################################---------------------------------------------------------] 600/911[##############################################################################################################################-----------------------------------------] 690/911[############################################################################################################################################---------------------------] 765/911[####################################################################################################################################################-------------------] 807/911[####################################################################################################################################################################---] 894/911

[KayLeung]

@swrobel This time really looks like another Yarn bug because the NodeJS console test in #4317 (comment) works well with rxvt.

[KayLeung]

@swrobel You could get it work with --color enabled

chalk/supports-color#67

[BYK]

@swrobel or @KayLeung you mind filing a new issue so someone can look at it?

[swrobel]

This is beyond me so I'll leave it to @KayLeung