Colorama edit fixes (v2) #124

Merged
merged 6 commits into from Jan 4, 2014

Conversation

Projects
None yet
2 participants
@MinchinWeb
Collaborator

MinchinWeb commented Jan 3, 2014

This builds off of #122 & #123.

Starting with #123, this makes two (simple) changes:

  • colorama needs to be initialized to work
  • colorama.Fore.CYAN, et al. are just shortcuts for the ANSI codes, which colorama then interprets. So we don't need separate coloring instructions for the two platforms

I've tested it on my end on Windows, and colored output and editing both now work.

Closes #121, closes #122, closes #123.

@maebert

This comment has been minimized.

Show comment
Hide comment
@maebert

maebert Jan 3, 2014

The point is, if you're not using windows, then colorama won't be installed, imported, or initialised. (See above: if 'win32' in sys.platform must be true to import colorama). On platforms that support ANSI control codes, we don't need a fat wrapper around them.

The point is, if you're not using windows, then colorama won't be installed, imported, or initialised. (See above: if 'win32' in sys.platform must be true to import colorama). On platforms that support ANSI control codes, we don't need a fat wrapper around them.

This comment has been minimized.

Show comment
Hide comment
@MinchinWeb

MinchinWeb Jan 3, 2014

Owner

But with coloama installed/initialized, you don't need this conditional test. The colorama color codes are just shortcuts for the ANSI codes.

colorama.Fore.CYAN == u"\033[36m"

Owner

MinchinWeb replied Jan 3, 2014

But with coloama installed/initialized, you don't need this conditional test. The colorama color codes are just shortcuts for the ANSI codes.

colorama.Fore.CYAN == u"\033[36m"

This comment has been minimized.

Show comment
Hide comment
@MinchinWeb

MinchinWeb Jan 3, 2014

Owner

Ultimately, the program works both with and without these two lines.I just thought the program would be a little cleaner without them.

Owner

MinchinWeb replied Jan 3, 2014

Ultimately, the program works both with and without these two lines.I just thought the program would be a little cleaner without them.

This comment has been minimized.

Show comment
Hide comment
@maebert

maebert Jan 3, 2014

Aaaah, wait, my bad. Are you saying that on Windows,

import colorama
colorama.init()
print "Hello \033[36mWorld\033[39m"

actually works and prints "World" in Cyan, whereas

import colorama
# colorama.init()
print "Hello \033[36mWorld\033[39m"

will literally print Hello \033[36mWorld\033[39m ?

Aaaah, wait, my bad. Are you saying that on Windows,

import colorama
colorama.init()
print "Hello \033[36mWorld\033[39m"

actually works and prints "World" in Cyan, whereas

import colorama
# colorama.init()
print "Hello \033[36mWorld\033[39m"

will literally print Hello \033[36mWorld\033[39m ?

This comment has been minimized.

Show comment
Hide comment
@MinchinWeb

MinchinWeb Jan 3, 2014

Owner

Yes and yes, exactly.

although in the second case the output is Hello ←[36mWorld←[39m (so basically the same. I have no idea why the arrows, but that's what shows up)

Owner

MinchinWeb replied Jan 3, 2014

Yes and yes, exactly.

although in the second case the output is Hello ←[36mWorld←[39m (so basically the same. I have no idea why the arrows, but that's what shows up)

@maebert

This comment has been minimized.

Show comment
Hide comment
@maebert

maebert Jan 3, 2014

Very good, thanks!

Very good, thanks!

This comment has been minimized.

Show comment
Hide comment
Owner

MinchinWeb replied Jan 3, 2014

:)

maebert added a commit that referenced this pull request Jan 4, 2014

@maebert maebert merged commit 92867a2 into maebert:master Jan 4, 2014

1 check passed

default The Travis CI build passed
Details
@maebert

This comment has been minimized.

Show comment
Hide comment
@maebert

maebert Jan 4, 2014

Owner

Alright, merging it in and deploying.

Owner

maebert commented Jan 4, 2014

Alright, merging it in and deploying.

@MinchinWeb MinchinWeb deleted the MinchinWeb:colorama-edit-fixes branch Jan 4, 2014

@MinchinWeb

This comment has been minimized.

Show comment
Hide comment
@MinchinWeb

MinchinWeb Jan 4, 2014

Collaborator

Thanks for all the good work!

Collaborator

MinchinWeb commented Jan 4, 2014

Thanks for all the good work!

@maebert

This comment has been minimized.

Show comment
Hide comment
@maebert

maebert Jan 4, 2014

Owner

No, thank you!

Owner

maebert commented Jan 4, 2014

No, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment