Skip to content

Comments

Removed const esc sequence#39

Open
andres-lowrie wants to merge 1 commit intojunyu-w:developfrom
andres-lowrie:consolidate-color-sequences
Open

Removed const esc sequence#39
andres-lowrie wants to merge 1 commit intojunyu-w:developfrom
andres-lowrie:consolidate-color-sequences

Conversation

@andres-lowrie
Copy link

Addresses: https://github.com/DrakeW/corgi/issues/34

Offloads all terminal coloring to fatih/color


How I did testing:

# From `develop`
go build
./corgi new -t exporttest
# Make a command

./corgi export exporttest
./corgi export exporttest -t shell
mv exporttest{,_beforechanges}.json
mv exporttest{,_beforechanges}

git checkout branch
go build
./corgi export exportest
./corgi export exportest -t shell

# There should be no differences
diff exporttest exporttest_beforechanges
diff exporttest.json exporttest_beforechanges.json

@andres-lowrie
Copy link
Author

andres-lowrie commented Oct 6, 2018

Forgot this bit:

I used ripgrep to see usage of the const escape sequence:

rg "SHELL_GREEN|SHELL_RED|SHELL_YELLOW|SHELL_NO_COLOR"

@junyu-w
Copy link
Owner

junyu-w commented Oct 18, 2018

@andres-lowrie thanks for the PR, I've reviewed and overall this looks solid!

@andres-lowrie andres-lowrie force-pushed the consolidate-color-sequences branch from b24f486 to 0f9ed4b Compare December 14, 2018 18:09
@andres-lowrie
Copy link
Author

Finally got some down time (yay holidays!!)

I believe I addressed the points you mentioned. I also rebased the branch for a cleaner commit message.

Lmk if this needs anything else!

Happy Holiday!

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.

2 participants