r-lib / ansistrings

Manipulation of ANSI colored strings
Other
9 stars 5 forks source link

`re_ansi()` pattern possibly incomplete #5

Open brodieG opened 7 years ago

brodieG commented 7 years ago

Consider:

xx <- crayon::make_style(rgb(2, 4, 2, maxColorValue=5), bg=TRUE, colors=256)
yy <- xx('hello world')
yy
## [1] "\033[48;5;114mhello world\033[49m"
gsub(re_ansi(), "", yy, perl=T)
## [1] "\033[48;5;114mhello world"
gsub(crayon:::ansi_regex, "", yy, perl=T)
## [1] "hello world"
brodieG commented 7 years ago

Seems like these lines should be:

two99            <- "[12]?[0-9]?[0-9]"   # match up to 299, should be 255
re_color         <- sprintf("3[0-7]|38;2;%s;%s;%s|38;5;%s", two99, two99, two99, two99)
re_bgcolor       <- sprintf("4[0-7]|48;2;%s;%s;%s|48;5;%s", two99, two99, two99, two99)

But I'm guessing you didn't just re-use the crayon::ansi_regex for a reason.

brodieG commented 7 years ago

Related:

zz <- '\033[31;47mHello\033[0m\n'
cat(zz)  # see in terminal
crayon::strip_style(zz)
## [1] "Hello\n"

re_match_all(zz, re_ansi())$start   # should be 31;47 or some such, I think
## [[1]]
## [1] ""

re_match_all(zz, re_ansi())$end
## [[1]]
## [1] "0"

This one combines the foreground and background commands into one escape sequence. I'm not super familiar with the ANSI escape sequence syntax or your reasons for not just re-using the existing regex (presumably for easier capture of start/end codes), so I don't want to propose a PR without additional info.

gaborcsardi commented 7 years ago

Yeah, I cannot use the exact same regex, because I need to capture the opening and closing tags. Nevertheless, you are right that it is buggy, I'll fix these in a minute....

gaborcsardi commented 7 years ago

The thing is, if we want to support sequences with multiple commands, like in \033[31;47m, then the whole logic will be much more difficult.

I think that eventually we should support these, but for now, I think we can ignore them. crayon never creates them, anyway.

I fixed the color regexes in c8afad7442799c0ec456865c87d744bf39c33aab

brodieG commented 7 years ago

Agreed, I was thinking the same. Documenting this is probably sufficient for now.