I made a couple of small changes for you:
bool supportsColor()
{
if(const char *env_p = std::getenv("TERM")) {
const char *const terms[] = {
"xterm", "xterm-256", "xterm-256color", "vt100",
"color", "ansi", "cygwin", "linux"};
// Change 1
for(auto const term: terms) {
if(std::strcmp(env_p, term) == 0) return true;
}
}
return false;
}
I swapped out your for loop with a range for loop which, apart from being all the rage, actually improve read-ability as well. Literally, for every term in terms. The compiler will optimise this in the same way as the original for loop.
To follow up on point 2 from Jerry Coffin Jerry Coffin's answer, using some template black magic allows us to reduce the number of overloads for operator<< to just one:
// Change 2
template<typename T>
using enable = typename std::enable_if
<
std::is_same<T, rang::style>::value ||
std::is_same<T, rang::fg>::value ||
std::is_same<T, rang::bg>::value,
std::ostream&
>::type;
template<typename T>
enable<T> operator<<(std::ostream& os, T const value)
{
...
}
Change 2 relies on a technique known as SFINAE (Substitution Failure Is Not An Error) whereby any substitutions that result in erroneous candidates for overload resolution, are simply removed. In our case, the return type is well-defined iff T has type rang::{style,fg,bg}.
One last change that I made is in the body of operator<<:
// Change 3
if(isAllowed) {
os << "\e[" << static_cast< int >(value) << "m";
}
return os;
While I am a proponent of the ternary operator, I question it's use here. The above if statement is more obvious. Since we return the ostream regardless of whether or not we used it, the result is the same as before.
You can see my modifications in action here sans the colourful output, unfortunately. Testing in my linux machine with g++ 4.8 yields a lovely colourful output.
I made a couple of small changes for you:
bool supportsColor()
{
if(const char *env_p = std::getenv("TERM")) {
const char *const terms[] = {
"xterm", "xterm-256", "xterm-256color", "vt100",
"color", "ansi", "cygwin", "linux"};
// Change 1
for(auto const term: terms) {
if(std::strcmp(env_p, term) == 0) return true;
}
}
return false;
}
I swapped out your for loop with a range for loop which, apart from being all the rage, actually improve read-ability as well. Literally, for every term in terms. The compiler will optimise this in the same way as the original for loop.
To follow up on point 2 from Jerry Coffin's answer, using some template black magic allows us to reduce the number of overloads for operator<< to just one:
// Change 2
template<typename T>
using enable = typename std::enable_if
<
std::is_same<T, rang::style>::value ||
std::is_same<T, rang::fg>::value ||
std::is_same<T, rang::bg>::value,
std::ostream&
>::type;
template<typename T>
enable<T> operator<<(std::ostream& os, T const value)
{
...
}
Change 2 relies on a technique known as SFINAE (Substitution Failure Is Not An Error) whereby any substitutions that result in erroneous candidates for overload resolution, are simply removed. In our case, the return type is well-defined iff T has type rang::{style,fg,bg}.
One last change that I made is in the body of operator<<:
// Change 3
if(isAllowed) {
os << "\e[" << static_cast< int >(value) << "m";
}
return os;
While I am a proponent of the ternary operator, I question it's use here. The above if statement is more obvious. Since we return the ostream regardless of whether or not we used it, the result is the same as before.
You can see my modifications in action here sans the colourful output, unfortunately. Testing in my linux machine with g++ 4.8 yields a lovely colourful output.
I made a couple of small changes for you:
bool supportsColor()
{
if(const char *env_p = std::getenv("TERM")) {
const char *const terms[] = {
"xterm", "xterm-256", "xterm-256color", "vt100",
"color", "ansi", "cygwin", "linux"};
// Change 1
for(auto const term: terms) {
if(std::strcmp(env_p, term) == 0) return true;
}
}
return false;
}
I swapped out your for loop with a range for loop which, apart from being all the rage, actually improve read-ability as well. Literally, for every term in terms. The compiler will optimise this in the same way as the original for loop.
To follow up on point 2 from Jerry Coffin's answer, using some template black magic allows us to reduce the number of overloads for operator<< to just one:
// Change 2
template<typename T>
using enable = typename std::enable_if
<
std::is_same<T, rang::style>::value ||
std::is_same<T, rang::fg>::value ||
std::is_same<T, rang::bg>::value,
std::ostream&
>::type;
template<typename T>
enable<T> operator<<(std::ostream& os, T const value)
{
...
}
Change 2 relies on a technique known as SFINAE (Substitution Failure Is Not An Error) whereby any substitutions that result in erroneous candidates for overload resolution, are simply removed. In our case, the return type is well-defined iff T has type rang::{style,fg,bg}.
One last change that I made is in the body of operator<<:
// Change 3
if(isAllowed) {
os << "\e[" << static_cast< int >(value) << "m";
}
return os;
While I am a proponent of the ternary operator, I question it's use here. The above if statement is more obvious. Since we return the ostream regardless of whether or not we used it, the result is the same as before.
You can see my modifications in action here sans the colourful output, unfortunately. Testing in my linux machine with g++ 4.8 yields a lovely colourful output.
I made a couple of small changes for you:
bool supportsColor()
{
if(const char *env_p = std::getenv("TERM")) {
const char *const terms[] = {
"xterm", "xterm-256", "xterm-256color", "vt100",
"color", "ansi", "cygwin", "linux"};
// Change 1
for(auto const term: terms) {
if(std::strcmp(env_p, term) == 0) return true;
}
}
return false;
}
I swapped out your for loop with a range for loop which, apart from being all the rage, actually improve read-ability as well. Literally, for every term in terms. The compiler will optimise this in the same way as the original for loop.
To follow up on point 2 from Jerry Coffin's answer, using some template black magic allows us to reduce the number of overloads for operator<< to just one:
// Change 2
template<typename T>
using enable = typename std::enable_if
<
std::is_same<T, rang::style>::value ||
std::is_same<T, rang::fg>::value ||
std::is_same<T, rang::bg>::value,
std::ostream&
>::type;
template<typename T>
enable<T> operator<<(std::ostream& os, T const value)
{
...
}
Change 2 relies on a technique known as SFINAE (Substitution Failure Is Not An Error) whereby any substitutions that result in erroneous candidates for overload resolution, are simply removed. In our case, the return type is well-defined iff T has type rang::{style,fg,bg}.
One last change that I made is in the body of operator<<:
// Change 3
if(isAllowed) {
os << "\e[" << static_cast< int >(value) << "m";
}
return os;
While I am a proponent of the ternary operator, I question it's use here. The above if statement is more obvious. Since we return the ostream regardless of whether or not we used it, the result is the same as before.
You can see my modifications in action here sans the colourful output, unfortunately. Testing in my linux machine with g++ 4.8 yields a lovely colourful output.