Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid ansi-escape replacing inside eclipse-pydev #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

x2nie
Copy link

@x2nie x2nie commented Oct 11, 2016

avoid conflicting colorama with the recommended eclipse plugin (ansi-escape) by pydev author that can enable color in pydev-console
it's make both running code in terminal & eclipse console can show the correct color
image
eclipse ansi-escape console = http://stackoverflow.com/questions/32196958/change-logging-colors-in-pydev-windows

avoid conflicting colorama with the recommended eclipse plugin (ansi-escape) by pydev author that can enable color in pydev-console
it's make both terminal & eclipse console can show color
@wiggin15
Copy link
Collaborator

Hi. I think the check will fit more neatly in AnsiToWin32.__init__ where the default strip/convert values are determined (I think we need convert=False and strip=False). Maybe we can add the pydev check to conversion_supported, or directly to the lines that determine strip and convert: https://github.com/tartley/colorama/blob/master/colorama/ansitowin32.py#L74 and https://github.com/tartley/colorama/blob/master/colorama/ansitowin32.py#L79

In addition there are pep8 issues with the comments:

colorama/ansitowin32.py:11:46: E261 at least two spaces before inline comment
colorama/ansitowin32.py:11:47: E262 inline comment should start with '# '

@tartley
Copy link
Owner

tartley commented Oct 13, 2020

Hey. FYI, yesterday I created a PR to test releases before we push them to PyPI. When that is merged, I'll be more confident about resuming merges and releases. I'll try to look at this PR soon. Thank you for creating it!

@tartley
Copy link
Owner

tartley commented Oct 7, 2021

I concur with wiggin's thoughts on this - the check needs to be done along with all the other similar checks in the place he suggests. Also, we'd need tests before we could merge this.

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

Successfully merging this pull request may close these issues.

3 participants