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

hex (or oct) or other stream manipulators will cause unexpected ANSI escape codes to be generated #87

Open
jogloran opened this issue Mar 5, 2018 · 11 comments
Assignees
Labels
Milestone

Comments

@jogloran
Copy link

jogloran commented Mar 5, 2018

Because setColor is implemented simply by inserting an int into the stream, the output will be affected by hex or oct or potentially other stream manipulators such as setw. Should the stream manipulator state be stored with ios.copyfmt and loaded after the escapes have been printed?

@kingseva
Copy link
Collaborator

kingseva commented Mar 6, 2018

You are right. We are somehow have forgot about format flags. We should store int to decimal string value using explicitly dec | left flags.

So instead of:

        os << "\033[" << static_cast<int>(value) << "m";

we should write:

        std::ostringstream oss;
        oss.flags(std::ios::dec | std::ios::left);
        oss << "\033[" << static_cast<int>(value) << "m";
        os << oss.str();

(in real code basic_ostringstream<CharT, will used)

It's better than storing flags explicitly using flags method:

        auto curflags = os.flags(); 
        os.flags(std::ios::dec | std::ios::left);
        os << "\033[" << static_cast<int>(value) << "m";
        os.flags(curflags); // restore previous flags

because user may set exceptions:

        sd::cout.exceptions ( std::ifstream::failbit | std::ifstream::badbit );

this may cause exception thrown before we restore the flags.

@agauniyal
Copy link
Owner

agauniyal commented Mar 6, 2018

@kingseva wouldn't os.str()'s string be affected by setw or some other string manipulator? what if some user creates custom stream manipulators which modifies the os.str() being sent to os << oss.str(); 🤔

@agauniyal
Copy link
Owner

saving current flag and restoring them can be safely done with a scope guard -

template <typename CharT, typename Traits>
class streamScopeGuard {
    std::basic_ostream<CharT, Traits> &os;
    std::ios_base::fmtflags flags;
    std::streamsize width;
    std::streamsize precision;

public:
    streamScopeGuard(std::basic_ostream<CharT, Traits> &_os)
        : os(_os),
          flags(os.flags()),
          width(os.width()),
          precision(os.precision())
    {
        os.flags(std::ios::dec | std::ios::left);
        os.width(0);
        os.precision(0);
    }
    ~streamScopeGuard()
    {
        os.flags(flags);
        os.width(width);
        os.precision(precision);
    }
};

template <typename CharT, typename Traits>
void green(std::basic_ostream<CharT, Traits> &os)
{
    streamScopeGuard guard(os);
    os << "\033[" << static_cast<int>(31) << "m";
    throw 1;
    return;
}

@kingseva
Copy link
Collaborator

kingseva commented Mar 6, 2018

saving current flag and restoring them can be safely done with a scope guard -
...

Yes. It's the same as boost::io::ios_all_saver. But requires more code.

@kingseva wouldn't os.str()'s string be affected by setw or some other string manipulator? what if some user creates custom stream manipulators which modifies the os.str() being sent to os << oss.str();

I don't know how to safely write to cout/wcout if user wants to replace standart operator for std::string/std::wstring. Maybe using cout.flush and than writing to corresponding FILE, using fwrite, but it's terrible. I think, If we can't aware of user operator overloading, we should not care about it.

@agauniyal
Copy link
Owner

Added for next-release.

@agauniyal agauniyal self-assigned this Mar 8, 2018
@agauniyal agauniyal added the bug label Mar 8, 2018
@agauniyal agauniyal added this to the Release 4.0 milestone Mar 8, 2018
@kingseva
Copy link
Collaborator

kingseva commented Mar 9, 2018

Another way of doing this is using unformatted output through write function. We also can create escaped string using snprintf/wsnprintf functions: http://rextester.com/UEV26874

@agauniyal
Copy link
Owner

This should solve the problem as well, much simpler but don't know about the cost.

template <typename CharT, typename Traits>
friend std::basic_ostream<CharT, Traits> &
operator<<(std::basic_ostream<CharT, Traits> &os, const action &obj)
{
	std::basic_ostream<CharT, Traits> tempStream(os.rdbuf());
	tempStream << 1 << 2.4 << 'a' << "Hello World";  // write data here
	return os;
}

@kingseva
Copy link
Collaborator

In this case we create temporary stream, the cost of it's creating depends from library call std::basic_ios::init. And than we call 3 times operator << that internally checks formatting flags and parse and write values to 'os' buffer. The reason why I'm propose it to place to another function is to avoid code duplication in setColor functions (for windows ansi and linux). Also user may wanted to serrialize all color calls into ansi string by hand. In this case it's usable to have ansi/wansi functions where same code for creating string been used.

@kingseva
Copy link
Collaborator

By the way, you give me an idea how to place this code in different function and use it anywhere where streams is used.

// Writing to any streambuf 
template <typename T,typename CharT, typename Traits = std::char_traits<CharT> >
inline void basic_ansi_buf(T const value,std::basic_streambuf<CharT, Traits>* buf)
{
    std::basic_ostream<CharT, Traits> tempStream(buf);
    tempStream << "\033[" << static_cast<int>(value) << "m";
}

It's easy to use in our operator << :

template <typename CharT, typename Traits>
friend std::basic_ostream<CharT, Traits> &
operator<<(std::basic_ostream<CharT, Traits> &os, const action &obj)
{
    basic_ansi_buf(value,os.rdbuf());
    return os;
}

We can use ostringstream and above function for std::string creation:

// Create ansi string using ostringstream and basic_ansi_buf
template<typename T,typename CharT, typename Traits = std::char_traits<CharT>, typename Alloc = std::allocator<CharT> >
inline std::basic_string<CharT,Traits,Alloc> basic_ansi_str(const T value)
{
    std::basic_ostringstream<CharT,Traits,Alloc> ostr;
    basic_ansi_buf(value,ostr.rdbuf());
    return ostr.str();
} 

And specializations for ansi/wansi:

template<typename T,typename CharT = char, typename Traits = std::char_traits<CharT>, typename Alloc = std::allocator<CharT> >
inline auto ansi(const T value) -> decltype(basic_ansi_str<T,CharT,Traits,Alloc>(value))
{
    return basic_ansi_str<T,CharT,Traits,Alloc>(value);
}   

template<typename T, typename CharT = wchar_t, typename Traits = std::char_traits<CharT>, typename Alloc = std::allocator<CharT> >
inline auto wansi(const T value) -> decltype(basic_ansi_str<T,CharT,Traits,Alloc>(value))
{
    return basic_ansi_str<T,CharT,Traits,Alloc>(value);
} 

In this approach we are explicitly depends from std::basic_streambuf. It may be bad if we wanted to port rang from STL to different library with another types of stream/string. I mean CharT[SIZE] and snprintf/wsnprintf is more common way, but it's more like C rather than C++.
Full code: http://rextester.com/CNBU10112

@agauniyal
Copy link
Owner

This is lot more cleaner, I'll revisit this way for version 5. For version 4 release, let's stay with saving stream flags method since my focus is on cursor operations 😄

@kingseva
Copy link
Collaborator

We also have minor and patch numbers in version 😄 Why not 4.1 ?

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

No branches or pull requests

3 participants