Better ANSI support detection.#43
Better ANSI support detection.#43kylef merged 3 commits intokylef:masterfrom djbe:feature/better-ansi-detection
Conversation
kylef
left a comment
There was a problem hiding this comment.
I have one comment above. Do you know if there is a radar for Xcode reporting to be an TTY but not supporting ANSI colour codes?
I'm not entirely sure on the behaviour, I didn't manage to find a formal specification for TTY/ANSI codes.
Sources/Error.swift
Outdated
| extension ANSIConvertible { | ||
| func print() { | ||
| if isatty(fileno(stderr)) != 0 { | ||
| // Check if Xcode Colors is installed and enabled. |
There was a problem hiding this comment.
From what I understand, it's not possible to use this plugin with Xcode 8? Since Commander is Swift 3. It's not possible to use Commander with XcodeColors so I don't think we need this check.
|
Not that I know of (I did a quick google and openradar search) |
Sources/Error.swift
Outdated
| return nil | ||
| } | ||
| return String(cString: value) | ||
| } |
There was a problem hiding this comment.
Nitpicking, but not sure it's worth having that private function anymore since XcodeColors isn't checked anymore and only the TERM env var has to be read
So using this directly for line 16 would suffice 😛
let termType = getenv("TERM").flatMap({ String(cString: $0) })There was a problem hiding this comment.
This won't work since getenv returns UnsafeMutablePointer not an optional.
There was a problem hiding this comment.
Right, so add the method back in again? It is clear what it does, and nicely wraps around unsafe pointer stuff.
There was a problem hiding this comment.
🤔 not sure to understand how the method we removed worked then… does if let value = getenv(…) magically do something special with UnsafeMutablePointer?
There was a problem hiding this comment.
Oh, seems UnsafeMutablePointer is actually force-unwrapped (!). So there is no optional, but if let would still work.
In SwiftGen we've just hit an issue with
ANSI supportdetection in the Xcode 8 console:SwiftGen/SwiftGen#221
While trying to find a solution, I found the Rainbow library, which has a bit more expansive tty detection:
https://github.com/onevcat/Rainbow/blob/master/Sources/OutputTarget.swift
This PR simply incorporates the extra detection into Commander.