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

Add uncolor method #54

Merged
merged 1 commit into from
Aug 15, 2017
Merged

Add uncolor method #54

merged 1 commit into from
Aug 15, 2017

Conversation

ypresto
Copy link
Contributor

@ypresto ypresto commented Mar 29, 2017

Similar method exists in Perl's Term:ANSIColor as uncolor().
It is useful when:

  • test code calls method which can return text with color.
  • calling terminal commands which can return text with color.

@ypresto ypresto changed the title Add strip_ascii_escape method Add strip_ansi_escape method Mar 29, 2017
@ku1ik ku1ik mentioned this pull request Apr 11, 2017
@ypresto
Copy link
Contributor Author

ypresto commented Jul 2, 2017

rebased!

Copy link
Collaborator

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

I think that the code is neat, and that the name is OK.


The name wrap_with_sgr has the SGR name in it; perhaps this stripping removes any control character, not only the graphics-related ones.

@ku1ik
Copy link
Owner

ku1ik commented Jul 3, 2017

Good observation @olleolleolle.

SGR stands for Set Graphics Rendition, and in addition to dealing with colors it also allows setting/unsetting other text attributes like bold, blinking, italic, underline and few others. This new method strips all ANSI escape sequences, not only SGR, so the name is appropriate in my opinion.

I hesitated to merge this because so far Rainbow only dealt with SGR seqs (escape seqs ending in m) and strip_ansi_escape clears the text of all ansi escape seqs (potentially cursor movement, erasing, scrolling, saving/restoring cursor position etc). But I think in most cases it would be used on colored text, so it's probably ok to have it in Rainbow.

@ku1ik
Copy link
Owner

ku1ik commented Jul 3, 2017

We can either merge it as it is now, or modify regex to only strip ones ending in m, and renaming the method to uncolor or strip_colors.

@ypresto
Copy link
Contributor Author

ypresto commented Jul 4, 2017

Changed to uncolor method..!

@ypresto ypresto changed the title Add strip_ansi_escape method Add uncolor method Jul 11, 2017
@ku1ik
Copy link
Owner

ku1ik commented Aug 15, 2017

Sorry, I've been busy recently. This looks good, let's merge it 👍

@ku1ik ku1ik merged commit 91501f2 into ku1ik:master Aug 15, 2017
@ypresto ypresto deleted the add-strip-ascii-escape branch August 17, 2017 14:13
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.

3 participants