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 ability to also recognize colons in ANSI escapes #167

Merged
merged 6 commits into from
Feb 20, 2022
Merged

Add ability to also recognize colons in ANSI escapes #167

merged 6 commits into from
Feb 20, 2022

Conversation

jaysonlarose
Copy link
Contributor

@jaysonlarose jaysonlarose commented Feb 16, 2022

This basically just treats colons and semicolons as exactly the same when it comes to parsing ANSI.

Fixes: #166

@ssbarnea ssbarnea added the bug This issue/PR relates to a bug. label Feb 16, 2022
@ssbarnea
Copy link
Member

Can you please provide some backing for making this change? After 20+ years of using ANSI that is the first time when I hear about using colons. A quick search on wikipedia. There is a section on 24-bit which mentions as being included in ODA, a commercial failure.

https://en.wikipedia.org/wiki/ANSI_escape_code

Basically what bug does this fix?

@jaysonlarose
Copy link
Contributor Author

See issue #166 for the full story.

@ssbarnea ssbarnea changed the title Super quick and dirty fix for issue #166 Add ability to also recognize colons in ANSI escapes Feb 16, 2022
@ssbarnea ssbarnea requested a review from hartwork February 16, 2022 12:40
Comment on lines 457 to 461
params = [
int(item)
for subitem in [x.split(":") for x in params.split(";")]
for item in subitem
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems to be way more complex than ideal and necessary. Would this approach work?:

In [7]: [int(v) for v in re.split("[:;]", "111;222:333;444")]
Out[7]: [111, 222, 333, 444]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably. I keep forgetting that re.split() exists.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jaysonlarose if you could simplify the code using re.split, I would consider it ready to merge. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@hartwork hartwork added this to the 1.7.1 milestone Feb 17, 2022
Copy link
Collaborator

@hartwork hartwork left a comment

Choose a reason for hiding this comment

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

@jaysonlarose thank you! 👍

@hartwork hartwork merged commit f856fde into pycontribs:main Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not support colon-delimited "flavor" of 24-bit "truecolor" escape sequences.
3 participants