Skip to content

cmd: color and emoji#595

Merged
bobheadxi merged 21 commits into
masterfrom
cmd/#147-color-and-emoji
Mar 2, 2019
Merged

cmd: color and emoji#595
bobheadxi merged 21 commits into
masterfrom
cmd/#147-color-and-emoji

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Mar 1, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

🎟️ Ticket(s): Closes #147

👷 Changes

  • rename output to just out
  • add decor.go with print wrappers and colour primitives (see cmd/core/utils/out/decor.go)
  • prettified a few common commands (notably init, ${remote} init, remote add, etc.)

Todos

🔦 Testing Instructions

make cli
./inertia

the output:

image

@bobheadxi bobheadxi requested a review from a team March 1, 2019 22:27
@bobheadxi bobheadxi added the pr: wip in progress but seeking feedback label Mar 1, 2019
@ghost ghost requested review from kimoantiqe, mRabitsky and nicholaschinjie March 1, 2019 22:27
@bobheadxi bobheadxi requested review from seifghazi, terryz21 and yaoharry and removed request for mRabitsky and nicholaschinjie March 1, 2019 22:28
@codecov

codecov Bot commented Mar 1, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #595 into master will increase coverage by 0.75%.
The diff coverage is 86.09%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #595      +/-   ##
=========================================
+ Coverage   55.66%   56.4%   +0.75%     
=========================================
  Files          68      69       +1     
  Lines        3229    3300      +71     
=========================================
+ Hits         1797    1861      +64     
- Misses       1198    1203       +5     
- Partials      234     236       +2
Impacted Files Coverage Δ
local/storage.go 26.16% <0%> (ø) ⬆️
cmd/core/utils/out/print.go 0% <0%> (ø)
cmd/core/utils/out/format.go 83.34% <55.56%> (ø)
client/bootstrap/bootstrap.go 72.59% <86.12%> (+0.49%) ⬆️
cmd/core/utils/input/input.go 76.82% <94.45%> (-0.11%) ⬇️
cmd/core/utils/out/decor.go 95.84% <95.84%> (ø)

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 6fb9e48...7359881. Read the comment docs.

yaoharry
yaoharry reviewed Mar 1, 2019
View reviewed changes

@yaoharry yaoharry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I'm curious on the behaviour in 16 bit terminals. Ex. Windows default cmd. I'll take a look at that before I put in approval.

@bobheadxi

bobheadxi commented Mar 1, 2019

Copy link
Copy Markdown
Member Author

@yaoharry would appreciate if you could take a look! the colour library (https://github.com/fatih/color#color--) says that windows is supported, but who knows (its been archived for a little bit)

I also just added a new flag --simple to disable all decorations (color and emoji), and the INERTIA_COLOR env var can be used to disable colours

@bobheadxi bobheadxi added pr: finalized needs review and final approval and removed pr: wip in progress but seeking feedback labels Mar 2, 2019
@bobheadxi bobheadxi requested a review from a team March 2, 2019 03:41
seifghazi
seifghazi approved these changes Mar 2, 2019
View reviewed changes

@seifghazi seifghazi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@yaoharry

yaoharry commented Mar 2, 2019

Copy link
Copy Markdown
Member

https://github.com/fatih/color#color--

Yeah looking at the library says it has Window support so thats nice. I think colors like cyan will default to blue on windows cmd, which imo is pretty ugly, but it's nice that we have a disable option.

@bobheadxi

bobheadxi commented Mar 2, 2019

Copy link
Copy Markdown
Member Author

I think colors like cyan will default to blue on windows cmd, which imo is pretty ugly

Rip lol well we can keep an eye on feedback and maybe change the colour or disable colours on windows or improve the toggling (right now you can only toggle emoji AND colour at once via the flag, to just toggle one you have to set an end variable which is a bit less ergonomic)

Thanks for the reviews everyone!

@bobheadxi bobheadxi merged commit 275f47b into master Mar 2, 2019
@bobheadxi bobheadxi deleted the cmd/#147-color-and-emoji branch March 2, 2019 09:00
This was referenced Mar 3, 2019
Closed
Merged
bobheadxi added a commit that referenced this pull request Mar 6, 2019
@bobheadxi
cmd: switch back to kyokomi/emoji (#599)
4d124ef 
follow-up to #595 and
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yaoharry yaoharry yaoharry approved these changes

@seifghazi seifghazi seifghazi approved these changes

@terryz21 terryz21 terryz21 approved these changes

@kimoantiqe kimoantiqe Awaiting requested review from kimoantiqe

Assignees

No one assigned

Labels

pr: finalized needs review and final approval

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

cmd: colors + emoji

4 participants

@bobheadxi @yaoharry @seifghazi @terryz21