heroku / heroku-apps

Heroku Core CLI Plugin for managing Heroku apps
https://cli.heroku.com
ISC License
19 stars 26 forks source link

Strip ANSI characters when fitting terminal width #218

Closed HaraldNordgren closed 6 years ago

HaraldNordgren commented 6 years ago

Fixing a bug introduced in https://github.com/heroku/heroku-apps/pull/216. I noticed that the previous fix was not taking all ANSI color sequences into account when calculating the lengths. The effect of this is that the table output will be still be aligned, but too narrow.

In the original implementation that stretched over heroku-apps and heroku-cli-util the ANSI stripping was done implicitly through https://github.com/heroku/heroku-cli-util/blob/v8.0.0/lib/table.js#L74. I forgot to include that when migrating that code to the current implementation.

Here I am creating a fix which is to call stripAnsi(formattedValue) when calculating the optimization lengths.

The rest of the PR is for increasing testability of the code. The older tests won't fail for this scenario since colors are turned off by default in the mock scenario.

codecov[bot] commented 6 years ago

Codecov Report

Merging #218 into master will increase coverage by 0.03%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #218      +/-   ##
==========================================
+ Coverage   95.79%   95.82%   +0.03%     
==========================================
  Files          76       76              
  Lines        1570     1582      +12     
  Branches      297      298       +1     
==========================================
+ Hits         1504     1516      +12     
  Misses         63       63              
  Partials        3        3
Impacted Files Coverage Δ
src/commands/releases/index.js 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 83b397d...a59397a. Read the comment docs.

HaraldNordgren commented 6 years ago

Ping @jdxcode @RasPhilCo @ransombriggs

HaraldNordgren commented 6 years ago

Ping @jdxcode @RasPhilCo @ransombriggs