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

Improve support for escape sequences to support Rainbow. #5

Closed
kiliankoe opened this issue Mar 1, 2016 · 26 comments · Fixed by #26
Closed

Improve support for escape sequences to support Rainbow. #5

kiliankoe opened this issue Mar 1, 2016 · 26 comments · Fixed by #26
Assignees

Comments

@kiliankoe
Copy link

Hi, just stumbled across SwiftyTextTable and wanted to use it for a project. Thanks for building it!

Unfortunately it seems SwiftTextTable is unable to calculate the correct size for a column when trying to apply additional information to the strings via something like onevcat/Rainbow.

I'm getting this for example when applying the color red to the string foo:

+--------------+-----+-----+
| foo | bar | baz |
+--------------+-----+-----+
| 1            | 2   | 3   |
| 11           |     | 33  |
| 111          | 222 | 333 |
+--------------+-----+-----+

I'm guessing it's because of this, seeing how color information in the terminal is represented through escape sequences that are counted as additional length in this case. Is there any way one could work around this?

@scottrhoyt
Copy link
Owner

Thanks for filing this!

Getting proper support for Rainbow was on the roadmap, so I'll take a look at this when a couple of free hours open up. It's quite possible that we can calculate string length in an alternative fashion (probably by stripping escape sequences). This also dovetails a bit with my intention to add multiline rows (basic support is sitting on a feature branch).

If you have any inclination to contribute a PR, I think the fix might be pretty straightforward, and I'd certainly welcome the help! Otherwise, I will probably get to it this week.

Going to rename this issue to better track it.

@scottrhoyt scottrhoyt changed the title Doesn't play nice with onevcat/Rainbow Improve support for escape sequences to support Rainbow. Mar 1, 2016
@scottrhoyt scottrhoyt added the bug label Mar 1, 2016
@kiliankoe
Copy link
Author

Thanks for the quick reply! I'll look into a fix for this, but can't promise anything. My knowledge of escape sequences is rather limited. But I'll definitely give it a shot!

@scottrhoyt
Copy link
Owner

No problem. Let me know if I can be of any assistance. My first hunch is that casting the values to NSString would probably open up some more powerful string functions. The functions for trimming characters in a character set would be promising.

@scottrhoyt
Copy link
Owner

So I looked a bit more into this. Unfortunately the escape sequences used by Rainbow to generate color and style information are a bit more complex than I was hoping, and they differ for platforms. While it would be possible to write a regex that stripped these for length calculations, the simplest way to accomplish this is probably just creating a dependency on Rainbow and using it's built-in functions for clearing color and styles to calculate length. I'll have to give some thought as to whether that would be a good idea, as I would like to avoid dependencies to keep the library as lightweight as possible.

@scottrhoyt
Copy link
Owner

@kiliankoe, I was able to come up with a regex to strip the console escapes for column width calculations. The results are available in this branch: https://github.com/scottrhoyt/SwiftyTextTable/tree/console_escape_stripping

Feel free to start using that as the public API won't change, but I may clean it up a bit before merging into master, which should be in the next day or two.

@kiliankoe
Copy link
Author

That's so cool, thank you for putting the work into this!

@scottrhoyt
Copy link
Owner

No problem! Having Rainbow support was needed IMO. In the future I'd like to explore potentially integrating table wide support for Rainbow. Also I'm going to add some protocols to make customizing the table display of values and displaying objects as rows easier.

@scottrhoyt
Copy link
Owner

Never mind, went ahead and cleaned it up before I went to bed. It's merged into master and a new release (0.2.2) is available now. Cheers!

@scottrhoyt
Copy link
Owner

@kiliankoe were you able to successfully create tables with color info from Rainbow?

@kiliankoe
Copy link
Author

I have, it seems to work just fine 😊 Thanks again!

screen shot 2016-03-15 at 09 36 21

@scottrhoyt
Copy link
Owner

Awesome!

@kiliankoe
Copy link
Author

kiliankoe commented Jul 23, 2017

Hi @scottrhoyt 👋

Unfortunately this issue seems to have returned. I've also tried it with https://github.com/mtynior/ColorizeSwift to be sure.

import SwiftyTextTable // 0.6.0
import Rainbow // 2.0.1

let columns = ["foo", "bar", "baz"].map { TextTableColumn(header: $0) }
var table = TextTable(columns: columns)

table.addRow(values: ["1", "2", "3"])
table.addRow(values: ["4".green, "5".yellow, "6".red])

print(table.render())

leads to

screen shot 2017-07-23 at 11 26 14

I've looked around the source but can't seem to find anything pertaining to this. Also the tests seem to pass just fine. Do you know what the reason for this could be?

@scottrhoyt
Copy link
Owner

Hmm. That's interesting @kiliankoe. Thanks for bringing it to my attention. Can you grab the escaped strings and post them here? My initial guess is that the the escape sequence used by Rainbow changed slightly and the stripping regex no longer catches it.

Also, double checking, are you seeing the incorrect behavior in terminal or in Xcode's console?

If you are interested in contributing at all, it would be awesome to throw those sequences into some more tests and rework the stripping regex to catch them.

@Roslund
Copy link
Contributor

Roslund commented Jan 14, 2018

I've had the same problem and I've "fixed" it in my fork. I did not make a pull request because it most likely breaks something else. I hope this can be of some use to you! 😄

Changes

First I've changed the stripping parten. It probably only matches the escape sequences I use.

-private let strippingPattern = "(?:\u{001B}\\[(?:[0-9]|;)+m)*(.*?)(?:\u{001B}\\[0m)+"
+private let strippingPattern = "\\\u{001B}\\[[0-1];[0-9][0-9]m"

Then I changed the with padding to use strippedLength() instead of count

 private extension String {
     func withPadding(count: Int) -> String {
-#if swift(>=3.2)
-        let length = self.count
-#else
-        let length = self.characters.count
-#endif
+        let length = self.strippedLength()
+
         if length < count {
             return self +
                 repeatElement(" ", count: count - length).joined()

Pictures

Before

After

Examples of the escape sequences I use for color

public enum StringColor: String {
    case black = "\u{001B}[0;30m"
    case red = "\u{001B}[0;31m"
    case boldRed = "\u{001B}[1;31m"
    case green = "\u{001B}[0;32m"
    case boldGreen = "\u{001B}[1;32m"
    case yellow = "\u{001B}[0;33m"
    case blue = "\u{001B}[0;34m"
    case boldBlue = "\u{001B}[1;34m"
    case magenta = "\u{001B}[0;35m"
    case cyan = "\u{001B}[0;36m"
    case white = "\u{001B}[0;37m"
}

@scottrhoyt
Copy link
Owner

Thanks for sharing this @enari ! 🥇 Are the unit tests still passing? If yes, I think if you can just come up with a unit test or two that fails on master but passes on your fork, then we should absolutely merge your fork. Let me know!

@Roslund
Copy link
Contributor

Roslund commented Feb 14, 2018

The unit test's are not passing. The strippingPatterns has to be combined.

@eneko
Copy link
Contributor

eneko commented Oct 12, 2018

@scottrhoyt Seems like this PR is ready to go. @enari 👏

@Roslund
Copy link
Contributor

Roslund commented Oct 16, 2018

@eneko: Yes, maybe it could use more testing, but it works and the unit tests are passing. However, @scottrhoyt does not seem to have been active lately. Maybe @jpsim can have a look and merge #26?

@jpsim jpsim closed this as completed in #26 Nov 14, 2018
@travispaul
Copy link

I think this issue is back.

import SwiftyTextTable //0.8.2
import Rainbow // 3.1.4


var testTable = TextTable(columns: [
    TextTableColumn(header: "col1"),
    TextTableColumn(header: "col2"),
    TextTableColumn(header: "col2")])

testTable.addRow(values: [1, "2".red, 3])
print(testTable.render())

example image

@Roslund
Copy link
Contributor

Roslund commented Apr 2, 2019

@travispaul: Could you find what escape sequence Rainbow uses for .red?
I think you can use print("2".red.debugDescription)

@travispaul
Copy link

Hi @Roslund, Looks like the escape sequence is "\u{1B}[31m2\u{1B}[0m"

@eneko
Copy link
Contributor

eneko commented Apr 3, 2019

@travispaul This code has been landed on master branch, however a new release has not been done since these updates were merged.

@jpsim could you please do a new 0.9.0 release?

@jpsim
Copy link
Collaborator

jpsim commented Apr 5, 2019

0.9.0 released here: https://github.com/scottrhoyt/SwiftyTextTable/releases/tag/0.9.0

We'll need @scottrhoyt to publish the release on CocoaPods because I don't have access rights to release it.

@travispaul
Copy link

@eneko and @jpsim works great now, thanks!

@travispaul
Copy link

Just an FYI, it looks like underline and bold suffer from the same issue (see Magnitude 6.4 on the 5th row):

red-bold image
red-underline image

print("X".red.underline.debugDescription) // "\u{1B}[31;4mX\u{1B}[0m"
print("X".red.bold.debugDescription)      // "\u{1B}[31;1mX\u{1B}[0m"

@Roslund
Copy link
Contributor

Roslund commented Aug 7, 2019

@travispaul I think a change to the regex should solve this. Try changing /Source/SwiftyTextTable/TextTable.swift line 13

-private let strippingPattern = "\\\u{001B}\\[([0-9][0-9]?m|[0-9](;[0-9]*)*m)"
+private let strippingPattern = "\\\u{(1B}\\[[0-9][0-9]?(;?[0-9]*)*m"

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

Successfully merging a pull request may close this issue.

6 participants