From 913f79a661a02be5791be9bdf2b8ee123579b51d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sun, 6 Mar 2022 19:20:53 +0100 Subject: [PATCH] Each reporter keeps its own colour implementation This opens path to per-reporter colour output customization, and fixes multiple issues with the old colour implementation. Under the old implementation, using Win32-backed colouring would always change the colour used by the console, even if the actual output was written elsewhere, such as a file passed by the `--out` flag. This will no longer happen, as the reporter's colour impl will check that the reporter's stream is pointed to console before trying to change the colours. POSIX/ANSI colour implementation suffered a similar-ish issue, in that it only wrote the colour escape codes into the default output stream, even if the reporter asking for colouring was actually writing to a completely different output stream. --- src/catch2/catch_session.cpp | 17 +- src/catch2/internal/catch_console_colour.cpp | 210 +++++++++--------- src/catch2/internal/catch_console_colour.hpp | 52 +++-- .../reporters/catch_reporter_combined_tu.cpp | 4 +- .../reporters/catch_reporter_common_base.cpp | 1 + .../reporters/catch_reporter_common_base.hpp | 7 +- .../reporters/catch_reporter_compact.cpp | 55 ++--- .../reporters/catch_reporter_console.cpp | 101 +++++---- .../reporters/catch_reporter_helpers.hpp | 2 + src/catch2/reporters/catch_reporter_tap.cpp | 38 ++-- .../IntrospectiveTests/Reporters.tests.cpp | 13 +- 11 files changed, 271 insertions(+), 229 deletions(-) diff --git a/src/catch2/catch_session.cpp b/src/catch2/catch_session.cpp index f20531e3..fc9b44c5 100644 --- a/src/catch2/catch_session.cpp +++ b/src/catch2/catch_session.cpp @@ -151,14 +151,16 @@ namespace Catch { getCurrentMutableContext().setConfig(m_config.get()); m_startupExceptions = true; - Colour colourGuard( Colour::Red ); - Catch::cerr() << "Errors occurred during startup!" << '\n'; + auto errStream = makeStream( "%stderr" ); + auto colourImpl = makeColourImpl( &config(), errStream.get() ); + auto guard = colourImpl->startColour( Colour::Red ); + errStream->stream() << "Errors occurred during startup!" << '\n'; // iterate over all exceptions and notify user for ( const auto& ex_ptr : exceptions ) { try { std::rethrow_exception(ex_ptr); } catch ( std::exception const& ex ) { - Catch::cerr() << TextFlow::Column( ex.what() ).indent(2) << '\n'; + errStream->stream() << TextFlow::Column( ex.what() ).indent(2) << '\n'; } } } @@ -194,12 +196,15 @@ namespace Catch { if( !result ) { config(); getCurrentMutableContext().setConfig(m_config.get()); - Catch::cerr() - << Colour( Colour::Red ) + auto errStream = makeStream( "%stderr" ); + auto colour = makeColourImpl( &config(), errStream.get() ); + + errStream->stream() + << colour->startColour( Colour::Red ) << "\nError(s) in input:\n" << TextFlow::Column( result.errorMessage() ).indent( 2 ) << "\n\n"; - Catch::cerr() << "Run with -? for usage\n\n" << std::flush; + errStream->stream() << "Run with -? for usage\n\n" << std::flush; return MaxExitCode; } diff --git a/src/catch2/internal/catch_console_colour.cpp b/src/catch2/internal/catch_console_colour.cpp index b3d583d2..4504eddc 100644 --- a/src/catch2/internal/catch_console_colour.cpp +++ b/src/catch2/internal/catch_console_colour.cpp @@ -24,23 +24,48 @@ #include namespace Catch { + + ColourImpl::~ColourImpl() = default; + + ColourImpl::ColourGuard ColourImpl::startColour( Colour::Code colourCode ) { + return ColourGuard(colourCode, this ); + } + namespace { + //! A do-nothing implementation of colour, used as fallback for unknown + //! platforms, and when the user asks to deactivate all colours. + class NoColourImpl : public ColourImpl { + public: + NoColourImpl( IStream const* stream ): ColourImpl( stream ) {} + static bool useColourOnPlatform() { return true; } - struct IColourImpl { - virtual ~IColourImpl() = default; - virtual void use( Colour::Code _colourCode ) = 0; + private: + void use( Colour::Code ) const override {} }; - struct NoColourImpl : IColourImpl { - void use( Colour::Code ) override {} + } // namespace - static IColourImpl* instance() { - static NoColourImpl s_instance; - return &s_instance; - } - }; + ColourImpl::ColourGuard::ColourGuard( Colour::Code code, + ColourImpl const* colour ): + m_colourImpl( colour ) { + m_colourImpl->use( code ); + } + ColourImpl::ColourGuard::ColourGuard( ColourGuard&& rhs ): + m_colourImpl( rhs.m_colourImpl ) { + rhs.m_moved = true; + } + ColourImpl::ColourGuard& + ColourImpl::ColourGuard::operator=( ColourGuard&& rhs ) { + m_colourImpl = rhs.m_colourImpl; + rhs.m_moved = true; + return *this; + } + ColourImpl::ColourGuard::~ColourGuard() { + if ( !m_moved ) { + m_colourImpl->use( Colour::None ); + } + } - } // anon namespace } // namespace Catch #if !defined( CATCH_CONFIG_COLOUR_NONE ) && !defined( CATCH_CONFIG_COLOUR_WINDOWS ) && !defined( CATCH_CONFIG_COLOUR_ANSI ) @@ -57,9 +82,10 @@ namespace Catch { namespace Catch { namespace { - class Win32ColourImpl : public IColourImpl { + class Win32ColourImpl : public ColourImpl { public: - Win32ColourImpl() { + Win32ColourImpl(IStream const* stream): + ColourImpl(stream) { CONSOLE_SCREEN_BUFFER_INFO csbiInfo; GetConsoleScreenBufferInfo( GetStdHandle( STD_OUTPUT_HANDLE ), &csbiInfo ); @@ -67,7 +93,16 @@ namespace { originalBackgroundAttributes = csbiInfo.wAttributes & ~( FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_BLUE | FOREGROUND_INTENSITY ); } - void use( Colour::Code _colourCode ) override { + static bool useColourOnPlatform() { return true; } + + private: + void use( Colour::Code _colourCode ) const override { + // Early exit if we are not writing to the console, because + // Win32 API can only change colour of the console. + if ( !m_stream->isStdout() ) { + return; + } + switch( _colourCode ) { case Colour::None: return setTextAttribute( originalForegroundAttributes ); case Colour::White: return setTextAttribute( FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_BLUE ); @@ -91,8 +126,7 @@ namespace { } } - private: - void setTextAttribute( WORD _textAttribute ) { + void setTextAttribute( WORD _textAttribute ) const { SetConsoleTextAttribute( GetStdHandle( STD_OUTPUT_HANDLE ), _textAttribute | originalBackgroundAttributes ); @@ -101,19 +135,6 @@ namespace { WORD originalBackgroundAttributes; }; - IColourImpl* platformColourInstance() { - static Win32ColourImpl s_instance; - - auto const* config = getCurrentContext().getConfig(); - UseColour colourMode = config? - config->useColour() : UseColour::Auto; - if( colourMode == UseColour::Auto ) - colourMode = UseColour::Yes; - return colourMode == UseColour::Yes - ? &s_instance - : NoColourImpl::instance(); - } - } // end anon namespace } // end namespace Catch @@ -128,9 +149,33 @@ namespace { // Thanks to Adam Strzelecki for original contribution // (http://github.com/nanoant) // https://github.com/philsquared/Catch/pull/131 - class PosixColourImpl : public IColourImpl { + class PosixColourImpl : public ColourImpl { public: - void use( Colour::Code _colourCode ) override { + PosixColourImpl( IStream const* stream ): ColourImpl( stream ) {} + + static bool useColourOnPlatform() { + ErrnoGuard _; // for isatty + return +# if defined( CATCH_PLATFORM_MAC ) || defined( CATCH_PLATFORM_IPHONE ) + !isDebuggerActive() && +# endif +# if !( defined( __DJGPP__ ) && defined( __STRICT_ANSI__ ) ) + isatty( STDOUT_FILENO ) +# else + false +# endif + ; + } + + private: + void use( Colour::Code _colourCode ) const override { + auto setColour = [&out = + m_stream->stream()]( char const* escapeCode ) { + // The escape sequence must be flushed to console, otherwise + // if stdin and stderr are intermixed, we'd get accidentally + // coloured output. + out << '\033' << escapeCode << std::flush; + }; switch( _colourCode ) { case Colour::None: case Colour::White: return setColour( "[0m" ); @@ -151,89 +196,52 @@ namespace { default: CATCH_INTERNAL_ERROR( "Unknown colour requested" ); } } - static IColourImpl* instance() { - static PosixColourImpl s_instance; - return &s_instance; - } - - private: - void setColour( const char* _escapeCode ) { - // The escape sequence must be flushed to console, otherwise if - // stdin and stderr are intermixed, we'd get accidentally coloured output. - getCurrentContext().getConfig()->defaultStream()->stream() - << '\033' << _escapeCode << std::flush; - } }; - bool useColourOnPlatform() { - return -#if defined(CATCH_PLATFORM_MAC) || defined(CATCH_PLATFORM_IPHONE) - !isDebuggerActive() && -#endif -#if !(defined(__DJGPP__) && defined(__STRICT_ANSI__)) - isatty(STDOUT_FILENO) -#else - false -#endif - ; - } - IColourImpl* platformColourInstance() { - ErrnoGuard guard; - auto const* config = getCurrentContext().getConfig(); - UseColour colourMode = config - ? config->useColour() - : UseColour::Auto; - if( colourMode == UseColour::Auto ) - colourMode = useColourOnPlatform() - ? UseColour::Yes - : UseColour::No; - return colourMode == UseColour::Yes - ? PosixColourImpl::instance() - : NoColourImpl::instance(); - } - } // end anon namespace } // end namespace Catch -#else // not Windows or ANSI /////////////////////////////////////////////// - -namespace Catch { - - static IColourImpl* platformColourInstance() { return NoColourImpl::instance(); } - -} // end namespace Catch - #endif // Windows/ ANSI/ None namespace Catch { - Colour::Colour( Code _colourCode ) { use( _colourCode ); } - Colour::Colour( Colour&& other ) noexcept { - m_moved = other.m_moved; - other.m_moved = true; - } - Colour& Colour::operator=( Colour&& other ) noexcept { - m_moved = other.m_moved; - other.m_moved = true; - return *this; - } + Detail::unique_ptr makeColourImpl(IConfig const* config, IStream const* stream) { + UseColour colourMode = config ? config->useColour() : UseColour::Auto; - Colour::~Colour(){ if( !m_moved ) use( None ); } - - void Colour::use( Code _colourCode ) { - static IColourImpl* impl = platformColourInstance(); - // Strictly speaking, this cannot possibly happen. - // However, under some conditions it does happen (see #1626), - // and this change is small enough that we can let practicality - // triumph over purity in this case. - if (impl != nullptr) { - impl->use( _colourCode ); + bool createPlatformInstance = false; + if ( colourMode == UseColour::No ) { + createPlatformInstance = false; } + + if ( colourMode == UseColour::Yes ) { + createPlatformInstance = true; + } + + if ( colourMode == UseColour::Auto ) { + createPlatformInstance = +#if defined( CATCH_CONFIG_COLOUR_ANSI ) + PosixColourImpl::useColourOnPlatform() +#elif defined( CATCH_CONFIG_COLOUR_WINDOWS ) + Win32ColourImpl::useColourOnPlatform() +#else + NoColourImpl::useColourOnPlatform() +#endif + ; + } + + if ( createPlatformInstance ) { + return +#if defined( CATCH_CONFIG_COLOUR_ANSI ) + Detail::make_unique(stream); +#elif defined( CATCH_CONFIG_COLOUR_WINDOWS ) + Detail::make_unique(stream); +#else + Detail::make_unique(stream); +#endif + } + return Detail::make_unique(stream); } - std::ostream& operator << ( std::ostream& os, Colour const& ) { - return os; - } } // end namespace Catch diff --git a/src/catch2/internal/catch_console_colour.hpp b/src/catch2/internal/catch_console_colour.hpp index 7b102013..e6a37967 100644 --- a/src/catch2/internal/catch_console_colour.hpp +++ b/src/catch2/internal/catch_console_colour.hpp @@ -8,10 +8,15 @@ #ifndef CATCH_CONSOLE_COLOUR_HPP_INCLUDED #define CATCH_CONSOLE_COLOUR_HPP_INCLUDED +#include + #include namespace Catch { + struct IConfig; + class IStream; + struct Colour { enum Code { None = 0, @@ -48,22 +53,41 @@ namespace Catch { SecondaryText = LightGrey, Headers = White }; - - // Use constructed object for RAII guard - Colour( Code _colourCode ); - Colour( Colour&& other ) noexcept; - Colour& operator=( Colour&& other ) noexcept; - ~Colour(); - - // Use static method for one-shot changes - static void use( Code _colourCode ); - - private: - bool m_moved = false; - - friend std::ostream& operator << (std::ostream& os, Colour const&); }; + class ColourImpl { + protected: + //! The associated stream of this ColourImpl instance + IStream const* m_stream; + public: + ColourImpl( IStream const* stream ): m_stream( stream ) {} + + class ColourGuard { + ColourImpl const* m_colourImpl; + bool m_moved = false; + friend std::ostream& operator<<(std::ostream& lhs, + ColourGuard const&) { + return lhs; + } + public: + ColourGuard( Colour::Code code, + ColourImpl const* colour ); + ColourGuard( ColourGuard const& rhs ) = delete; + ColourGuard& operator=( ColourGuard const& rhs ) = delete; + ColourGuard( ColourGuard&& rhs ); + ColourGuard& operator=( ColourGuard&& rhs ); + ~ColourGuard(); + }; + + virtual ~ColourImpl(); // = default + ColourGuard startColour( Colour::Code colourCode ); + + private: + virtual void use( Colour::Code colourCode ) const = 0; + }; + + //! Provides ColourImpl based on global config and target compilation platform + Detail::unique_ptr makeColourImpl( IConfig const* config, IStream const* stream ); } // end namespace Catch diff --git a/src/catch2/reporters/catch_reporter_combined_tu.cpp b/src/catch2/reporters/catch_reporter_combined_tu.cpp index 3d82f729..381d0743 100644 --- a/src/catch2/reporters/catch_reporter_combined_tu.cpp +++ b/src/catch2/reporters/catch_reporter_combined_tu.cpp @@ -174,7 +174,7 @@ namespace Catch { out << pluralise(tags.size(), "tag"_sr) << "\n\n" << std::flush; } - void defaultListTests(std::ostream& out, std::vector const& tests, bool isFiltered, Verbosity verbosity) { + void defaultListTests(std::ostream& out, ColourImpl* streamColour, std::vector const& tests, bool isFiltered, Verbosity verbosity) { // We special case this to provide the equivalent of old // `--list-test-names-only`, which could then be used by the // `--input-file` option. @@ -194,7 +194,7 @@ namespace Catch { Colour::Code colour = testCaseInfo.isHidden() ? Colour::SecondaryText : Colour::None; - Colour colourGuard(colour); + auto colourGuard = streamColour->startColour( colour ); out << TextFlow::Column(testCaseInfo.name).initialIndent(2).indent(4) << '\n'; if (verbosity >= Verbosity::High) { diff --git a/src/catch2/reporters/catch_reporter_common_base.cpp b/src/catch2/reporters/catch_reporter_common_base.cpp index 18a014fc..d49c2edb 100644 --- a/src/catch2/reporters/catch_reporter_common_base.cpp +++ b/src/catch2/reporters/catch_reporter_common_base.cpp @@ -18,6 +18,7 @@ namespace Catch { void ReporterBase::listTests(std::vector const& tests) { defaultListTests(m_stream, + m_colour.get(), tests, m_config->hasTestFilters(), m_config->verbosity()); diff --git a/src/catch2/reporters/catch_reporter_common_base.hpp b/src/catch2/reporters/catch_reporter_common_base.hpp index b7610a93..b69e5311 100644 --- a/src/catch2/reporters/catch_reporter_common_base.hpp +++ b/src/catch2/reporters/catch_reporter_common_base.hpp @@ -10,6 +10,7 @@ #include #include +#include namespace Catch { /** @@ -29,14 +30,14 @@ namespace Catch { //! Cached output stream from `m_wrapped_stream` to reduce //! number of indirect calls needed to write output. std::ostream& m_stream; - - + Detail::unique_ptr m_colour; public: ReporterBase( ReporterConfig const& config ): IEventListener( config.fullConfig() ), m_wrapped_stream( config.stream() ), - m_stream( m_wrapped_stream->stream() ) {} + m_stream( m_wrapped_stream->stream() ), + m_colour( makeColourImpl( config.fullConfig(), m_wrapped_stream ) ) {} /** diff --git a/src/catch2/reporters/catch_reporter_compact.cpp b/src/catch2/reporters/catch_reporter_compact.cpp index 6c77003f..2f0a20b0 100644 --- a/src/catch2/reporters/catch_reporter_compact.cpp +++ b/src/catch2/reporters/catch_reporter_compact.cpp @@ -18,9 +18,6 @@ namespace { - // Colour::LightGrey - constexpr Catch::Colour::Code dimColour() { return Catch::Colour::FileName; } - constexpr Catch::StringRef bothOrAll( std::uint64_t count ) { switch (count) { case 1: @@ -38,6 +35,9 @@ namespace { namespace Catch { namespace { + // Colour::LightGrey + static constexpr Colour::Code compactDimColour = Colour::FileName; + #ifdef CATCH_PLATFORM_MAC static constexpr Catch::StringRef compactFailedString = "FAILED"_sr; static constexpr Catch::StringRef compactPassedString = "PASSED"_sr; @@ -52,11 +52,11 @@ namespace { // - white: Passed [both/all] N test cases (no assertions). // - red: Failed N tests cases, failed M assertions. // - green: Passed [both/all] N tests cases with M assertions. -void printTotals(std::ostream& out, const Totals& totals) { +void printTotals(std::ostream& out, const Totals& totals, ColourImpl* colourImpl) { if (totals.testCases.total() == 0) { out << "No tests ran."; } else if (totals.testCases.failed == totals.testCases.total()) { - Colour colour(Colour::ResultError); + auto guard = colourImpl->startColour( Colour::ResultError ); const StringRef qualify_assertions_failed = totals.assertions.failed == totals.assertions.total() ? bothOrAll(totals.assertions.failed) : StringRef{}; @@ -71,13 +71,11 @@ void printTotals(std::ostream& out, const Totals& totals) { << pluralise(totals.testCases.total(), "test case"_sr) << " (no assertions)."; } else if (totals.assertions.failed) { - Colour colour(Colour::ResultError); - out << + out << colourImpl->startColour( Colour::ResultError ) << "Failed " << pluralise(totals.testCases.failed, "test case"_sr) << ", " "failed " << pluralise(totals.assertions.failed, "assertion"_sr) << '.'; } else { - Colour colour(Colour::ResultSuccess); - out << + out << colourImpl->startColour( Colour::ResultSuccess ) << "Passed " << bothOrAll(totals.testCases.passed) << pluralise(totals.testCases.passed, "test case"_sr) << " with " << pluralise(totals.assertions.passed, "assertion"_sr) << '.'; @@ -89,12 +87,14 @@ class AssertionPrinter { public: AssertionPrinter& operator= (AssertionPrinter const&) = delete; AssertionPrinter(AssertionPrinter const&) = delete; - AssertionPrinter(std::ostream& _stream, AssertionStats const& _stats, bool _printInfoMessages) + AssertionPrinter(std::ostream& _stream, AssertionStats const& _stats, bool _printInfoMessages, ColourImpl* colourImpl_) : stream(_stream) , result(_stats.assertionResult) , messages(_stats.infoMessages) , itMessage(_stats.infoMessages.begin()) - , printInfoMessages(_printInfoMessages) {} + , printInfoMessages(_printInfoMessages) + , colourImpl(colourImpl_) + {} void print() { printSourceInfo(); @@ -166,16 +166,13 @@ public: private: void printSourceInfo() const { - Colour colourGuard(Colour::FileName); - stream << result.getSourceInfo() << ':'; + stream << colourImpl->startColour( Colour::FileName ) + << result.getSourceInfo() << ':'; } void printResultType(Colour::Code colour, StringRef passOrFail) const { if (!passOrFail.empty()) { - { - Colour colourGuard(colour); - stream << ' ' << passOrFail; - } + stream << colourImpl->startColour(colour) << ' ' << passOrFail; stream << ':'; } } @@ -188,8 +185,7 @@ private: if (result.hasExpression()) { stream << ';'; { - Colour colour(dimColour()); - stream << " expression was:"; + stream << colourImpl->startColour(compactDimColour) << " expression was:"; } printOriginalExpression(); } @@ -203,10 +199,7 @@ private: void printReconstructedExpression() const { if (result.hasExpandedExpression()) { - { - Colour colour(dimColour()); - stream << " for: "; - } + stream << colourImpl->startColour(compactDimColour) << " for: "; stream << result.getExpandedExpression(); } } @@ -218,25 +211,22 @@ private: } } - void printRemainingMessages(Colour::Code colour = dimColour()) { + void printRemainingMessages(Colour::Code colour = compactDimColour) { if (itMessage == messages.end()) return; const auto itEnd = messages.cend(); const auto N = static_cast(std::distance(itMessage, itEnd)); - { - Colour colourGuard(colour); - stream << " with " << pluralise(N, "message"_sr) << ':'; - } + stream << colourImpl->startColour( colour ) << " with " + << pluralise( N, "message"_sr ) << ':'; while (itMessage != itEnd) { // If this assertion is a warning ignore any INFO messages if (printInfoMessages || itMessage->type != ResultWas::Info) { printMessage(); if (itMessage != itEnd) { - Colour colourGuard(dimColour()); - stream << " and"; + stream << colourImpl->startColour(compactDimColour) << " and"; } continue; } @@ -250,6 +240,7 @@ private: std::vector messages; std::vector::const_iterator itMessage; bool printInfoMessages; + ColourImpl* colourImpl; }; } // anon namespace @@ -274,7 +265,7 @@ private: printInfoMessages = false; } - AssertionPrinter printer( m_stream, _assertionStats, printInfoMessages ); + AssertionPrinter printer( m_stream, _assertionStats, printInfoMessages, m_colour.get() ); printer.print(); m_stream << '\n' << std::flush; @@ -288,7 +279,7 @@ private: } void CompactReporter::testRunEnded( TestRunStats const& _testRunStats ) { - printTotals( m_stream, _testRunStats.totals ); + printTotals( m_stream, _testRunStats.totals, m_colour.get() ); m_stream << "\n\n" << std::flush; StreamingReporterBase::testRunEnded( _testRunStats ); } diff --git a/src/catch2/reporters/catch_reporter_console.cpp b/src/catch2/reporters/catch_reporter_console.cpp index e79d6a63..42a71d56 100644 --- a/src/catch2/reporters/catch_reporter_console.cpp +++ b/src/catch2/reporters/catch_reporter_console.cpp @@ -45,13 +45,14 @@ class ConsoleAssertionPrinter { public: ConsoleAssertionPrinter& operator= (ConsoleAssertionPrinter const&) = delete; ConsoleAssertionPrinter(ConsoleAssertionPrinter const&) = delete; - ConsoleAssertionPrinter(std::ostream& _stream, AssertionStats const& _stats, bool _printInfoMessages) + ConsoleAssertionPrinter(std::ostream& _stream, AssertionStats const& _stats, ColourImpl* colourImpl_, bool _printInfoMessages) : stream(_stream), stats(_stats), result(_stats.assertionResult), colour(Colour::None), message(result.getMessage()), messages(_stats.infoMessages), + colourImpl(colourImpl_), printInfoMessages(_printInfoMessages) { switch (result.getResultType()) { case ResultWas::Ok: @@ -134,23 +135,22 @@ public: private: void printResultType() const { if (!passOrFail.empty()) { - Colour colourGuard(colour); - stream << passOrFail << ":\n"; + stream << colourImpl->startColour(colour) << passOrFail << ":\n"; } } void printOriginalExpression() const { if (result.hasExpression()) { - Colour colourGuard(Colour::OriginalExpression); - stream << " "; - stream << result.getExpressionInMacro(); - stream << '\n'; + stream << colourImpl->startColour( Colour::OriginalExpression ) + << " " << result.getExpressionInMacro() << '\n'; } } void printReconstructedExpression() const { if (result.hasExpandedExpression()) { stream << "with expansion:\n"; - Colour colourGuard(Colour::ReconstructedExpression); - stream << TextFlow::Column(result.getExpandedExpression()).indent(2) << '\n'; + stream << colourImpl->startColour( Colour::ReconstructedExpression ) + << TextFlow::Column( result.getExpandedExpression() ) + .indent( 2 ) + << '\n'; } } void printMessage() const { @@ -163,8 +163,8 @@ private: } } void printSourceInfo() const { - Colour colourGuard(Colour::FileName); - stream << result.getSourceInfo() << ": "; + stream << colourImpl->startColour( Colour::FileName ) + << result.getSourceInfo() << ": "; } std::ostream& stream; @@ -175,6 +175,7 @@ private: std::string messageLabel; std::string message; std::vector messages; + ColourImpl* colourImpl; bool printInfoMessages; }; @@ -403,7 +404,7 @@ void ConsoleReporter::assertionEnded(AssertionStats const& _assertionStats) { lazyPrint(); - ConsoleAssertionPrinter printer(m_stream, _assertionStats, includeResults); + ConsoleAssertionPrinter printer(m_stream, _assertionStats, m_colour.get(), includeResults); printer.print(); m_stream << '\n' << std::flush; } @@ -417,7 +418,7 @@ void ConsoleReporter::sectionEnded(SectionStats const& _sectionStats) { m_tablePrinter->close(); if (_sectionStats.missingAssertions) { lazyPrint(); - Colour colour(Colour::ResultError); + auto guard = m_colour->startColour( Colour::ResultError ); if (m_sectionStack.size() > 1) m_stream << "\nNo assertions in section"; else @@ -475,7 +476,7 @@ void ConsoleReporter::benchmarkEnded(BenchmarkStats<> const& stats) { } void ConsoleReporter::benchmarkFailed( StringRef error ) { - Colour colour(Colour::Red); + auto guard = m_colour->startColour( Colour::Red ); (*m_tablePrinter) << "Benchmark failed (" << error << ')' << ColumnBreak() << RowBreak(); @@ -514,13 +515,13 @@ void ConsoleReporter::lazyPrintWithoutClosingBenchmarkTable() { } } void ConsoleReporter::lazyPrintRunInfo() { - m_stream << '\n' << lineOfChars('~') << '\n'; - Colour colour(Colour::SecondaryText); - m_stream << currentTestRunInfo.name - << " is a Catch2 v" << libraryVersion() << " host application.\n" - << "Run with -? for options\n\n"; - - m_stream << "Randomness seeded to: " << m_config->rngSeed() << "\n\n"; + m_stream << '\n' + << lineOfChars( '~' ) << '\n' + << m_colour->startColour( Colour::SecondaryText ) + << currentTestRunInfo.name << " is a Catch2 v" << libraryVersion() + << " host application.\n" + << "Run with -? for options\n\n" + << "Randomness seeded to: " << m_config->rngSeed() << "\n\n"; m_testRunInfoPrinted = true; } @@ -529,7 +530,7 @@ void ConsoleReporter::printTestCaseAndSectionHeader() { printOpenHeader(currentTestCaseInfo->name); if (m_sectionStack.size() > 1) { - Colour colourGuard(Colour::Headers); + auto guard = m_colour->startColour( Colour::Headers ); auto it = m_sectionStack.begin() + 1, // Skip first section (test case) @@ -541,10 +542,10 @@ void ConsoleReporter::printTestCaseAndSectionHeader() { SourceLineInfo lineInfo = m_sectionStack.back().lineInfo; - m_stream << lineOfChars('-') << '\n'; - Colour colourGuard(Colour::FileName); - m_stream << lineInfo << '\n'; - m_stream << lineOfChars('.') << "\n\n" << std::flush; + m_stream << lineOfChars( '-' ) << '\n' + << m_colour->startColour( Colour::FileName ) << lineInfo << '\n' + << lineOfChars( '.' ) << "\n\n" + << std::flush; } void ConsoleReporter::printClosedHeader(std::string const& _name) { @@ -554,7 +555,7 @@ void ConsoleReporter::printClosedHeader(std::string const& _name) { void ConsoleReporter::printOpenHeader(std::string const& _name) { m_stream << lineOfChars('-') << '\n'; { - Colour colourGuard(Colour::Headers); + auto guard = m_colour->startColour( Colour::Headers ); printHeaderString(_name); } } @@ -619,9 +620,11 @@ struct SummaryColumn { void ConsoleReporter::printTotals( Totals const& totals ) { if (totals.testCases.total() == 0) { - m_stream << Colour(Colour::Warning) << "No tests ran\n"; + m_stream << m_colour->startColour( Colour::Warning ) + << "No tests ran\n"; } else if (totals.assertions.total() > 0 && totals.testCases.allPassed()) { - m_stream << Colour(Colour::ResultSuccess) << "All tests passed"; + m_stream << m_colour->startColour( Colour::ResultSuccess ) + << "All tests passed"; m_stream << " (" << pluralise(totals.assertions.passed, "assertion"_sr) << " in " << pluralise(totals.testCases.passed, "test case"_sr) << ')' @@ -648,17 +651,19 @@ void ConsoleReporter::printTotals( Totals const& totals ) { } void ConsoleReporter::printSummaryRow(StringRef label, std::vector const& cols, std::size_t row) { for (auto col : cols) { - std::string value = col.rows[row]; + std::string const& value = col.rows[row]; if (col.label.empty()) { m_stream << label << ": "; - if (value != "0") + if ( value != "0" ) { m_stream << value; - else - m_stream << Colour(Colour::Warning) << "- none -"; + } else { + m_stream << m_colour->startColour( Colour::Warning ) + << "- none -"; + } } else if (value != "0") { - m_stream << Colour(Colour::LightGrey) << " | "; - m_stream << Colour(col.colour) - << value << ' ' << col.label; + m_stream << m_colour->startColour( Colour::LightGrey ) << " | " + << m_colour->startColour( col.colour ) << value << ' ' + << col.label; } } m_stream << '\n'; @@ -674,14 +679,20 @@ void ConsoleReporter::printTotalsDivider(Totals const& totals) { while (failedRatio + failedButOkRatio + passedRatio > CATCH_CONFIG_CONSOLE_WIDTH - 1) findMax(failedRatio, failedButOkRatio, passedRatio)--; - m_stream << Colour(Colour::Error) << std::string(failedRatio, '='); - m_stream << Colour(Colour::ResultExpectedFailure) << std::string(failedButOkRatio, '='); - if (totals.testCases.allPassed()) - m_stream << Colour(Colour::ResultSuccess) << std::string(passedRatio, '='); - else - m_stream << Colour(Colour::Success) << std::string(passedRatio, '='); + m_stream << m_colour->startColour( Colour::Error ) + << std::string( failedRatio, '=' ) + << m_colour->startColour( Colour::ResultExpectedFailure ) + << std::string( failedButOkRatio, '=' ); + if ( totals.testCases.allPassed() ) { + m_stream << m_colour->startColour( Colour::ResultSuccess ) + << std::string( passedRatio, '=' ); + } else { + m_stream << m_colour->startColour( Colour::Success ) + << std::string( passedRatio, '=' ); + } } else { - m_stream << Colour(Colour::Warning) << std::string(CATCH_CONFIG_CONSOLE_WIDTH - 1, '='); + m_stream << m_colour->startColour( Colour::Warning ) + << std::string( CATCH_CONFIG_CONSOLE_WIDTH - 1, '=' ); } m_stream << '\n'; } @@ -691,8 +702,8 @@ void ConsoleReporter::printSummaryDivider() { void ConsoleReporter::printTestFilters() { if (m_config->testSpec().hasFilters()) { - Colour guard(Colour::BrightYellow); - m_stream << "Filters: " << serializeFilters(m_config->getTestsOrTags()) << '\n'; + m_stream << m_colour->startColour( Colour::BrightYellow ) << "Filters: " + << serializeFilters( m_config->getTestsOrTags() ) << '\n'; } } diff --git a/src/catch2/reporters/catch_reporter_helpers.hpp b/src/catch2/reporters/catch_reporter_helpers.hpp index d16d92f2..8a8051d7 100644 --- a/src/catch2/reporters/catch_reporter_helpers.hpp +++ b/src/catch2/reporters/catch_reporter_helpers.hpp @@ -19,6 +19,7 @@ namespace Catch { struct IConfig; class TestCaseHandle; + class ColourImpl; // Returns double formatted as %.3f (format expected on output) std::string getFormattedDuration( double duration ); @@ -67,6 +68,7 @@ namespace Catch { * `--list-test-names-only` option, for people who used it in integrations. */ void defaultListTests( std::ostream& out, + ColourImpl* streamColour, std::vector const& tests, bool isFiltered, Verbosity verbosity ); diff --git a/src/catch2/reporters/catch_reporter_tap.cpp b/src/catch2/reporters/catch_reporter_tap.cpp index a887ed6c..86a862d1 100644 --- a/src/catch2/reporters/catch_reporter_tap.cpp +++ b/src/catch2/reporters/catch_reporter_tap.cpp @@ -20,18 +20,20 @@ namespace Catch { // Making older compiler happy is hard. static constexpr StringRef tapFailedString = "not ok"_sr; static constexpr StringRef tapPassedString = "ok"_sr; + static constexpr Colour::Code tapDimColour = Colour::FileName; class TapAssertionPrinter { public: TapAssertionPrinter& operator= (TapAssertionPrinter const&) = delete; TapAssertionPrinter(TapAssertionPrinter const&) = delete; - TapAssertionPrinter(std::ostream& _stream, AssertionStats const& _stats, std::size_t _counter) + TapAssertionPrinter(std::ostream& _stream, AssertionStats const& _stats, std::size_t _counter, ColourImpl* colour_) : stream(_stream) , result(_stats.assertionResult) , messages(_stats.infoMessages) , itMessage(_stats.infoMessages.begin()) , printInfoMessages(true) - , counter(_counter) {} + , counter(_counter) + , colourImpl( colour_ ) {} void print() { itMessage = messages.begin(); @@ -104,11 +106,9 @@ namespace Catch { } private: - static Colour::Code dimColour() { return Colour::FileName; } - void printSourceInfo() const { - Colour colourGuard(dimColour()); - stream << result.getSourceInfo() << ':'; + stream << colourImpl->startColour( tapDimColour ) + << result.getSourceInfo() << ':'; } void printResultType(StringRef passOrFail) const { @@ -124,10 +124,8 @@ namespace Catch { void printExpressionWas() { if (result.hasExpression()) { stream << ';'; - { - Colour colour(dimColour()); - stream << " expression was:"; - } + stream << colourImpl->startColour( tapDimColour ) + << " expression was:"; printOriginalExpression(); } } @@ -140,10 +138,8 @@ namespace Catch { void printReconstructedExpression() const { if (result.hasExpandedExpression()) { - { - Colour colour(dimColour()); - stream << " for: "; - } + stream << colourImpl->startColour( tapDimColour ) << " for: "; + std::string expr = result.getExpandedExpression(); std::replace(expr.begin(), expr.end(), '\n', ' '); stream << expr; @@ -157,7 +153,7 @@ namespace Catch { } } - void printRemainingMessages(Colour::Code colour = dimColour()) { + void printRemainingMessages(Colour::Code colour = tapDimColour) { if (itMessage == messages.end()) { return; } @@ -166,18 +162,15 @@ namespace Catch { std::vector::const_iterator itEnd = messages.end(); const std::size_t N = static_cast(std::distance(itMessage, itEnd)); - { - Colour colourGuard(colour); - stream << " with " << pluralise(N, "message"_sr) << ':'; - } + stream << colourImpl->startColour( colour ) << " with " + << pluralise( N, "message"_sr ) << ':'; for (; itMessage != itEnd; ) { // If this assertion is a warning ignore any INFO messages if (printInfoMessages || itMessage->type != ResultWas::Info) { stream << " '" << itMessage->message << '\''; if (++itMessage != itEnd) { - Colour colourGuard(dimColour()); - stream << " and"; + stream << colourImpl->startColour(tapDimColour) << " and"; } } } @@ -190,6 +183,7 @@ namespace Catch { std::vector::const_iterator itMessage; bool printInfoMessages; std::size_t counter; + ColourImpl* colourImpl; }; } // End anonymous namespace @@ -202,7 +196,7 @@ namespace Catch { ++counter; m_stream << "# " << currentTestCaseInfo->name << '\n'; - TapAssertionPrinter printer(m_stream, _assertionStats, counter); + TapAssertionPrinter printer(m_stream, _assertionStats, counter, m_colour.get()); printer.print(); m_stream << '\n' << std::flush; diff --git a/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp b/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp index a2416042..dcb186b6 100644 --- a/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/Reporters.tests.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -40,11 +41,11 @@ TEST_CASE( "The default listing implementation write to provided stream", using Catch::Matchers::ContainsSubstring; using namespace std::string_literals; - std::stringstream sstream; + StringIStream sstream; SECTION( "Listing tags" ) { std::vector tags(1); tags[0].add("fakeTag"_catch_sr); - Catch::defaultListTags(sstream, tags, false); + Catch::defaultListTags(sstream.stream(), tags, false); auto listingString = sstream.str(); REQUIRE_THAT(listingString, ContainsSubstring("[fakeTag]"s)); @@ -52,7 +53,7 @@ TEST_CASE( "The default listing implementation write to provided stream", SECTION( "Listing reporters" ) { std::vector reporters( { { "fake reporter", "fake description" } } ); - Catch::defaultListReporters(sstream, reporters, Catch::Verbosity::Normal); + Catch::defaultListReporters(sstream.stream(), reporters, Catch::Verbosity::Normal); auto listingString = sstream.str(); REQUIRE_THAT(listingString, ContainsSubstring("fake reporter"s)); @@ -63,7 +64,11 @@ TEST_CASE( "The default listing implementation write to provided stream", { "fake test name"_catch_sr, "[fakeTestTag]"_catch_sr }, { "fake-file.cpp", 123456789 } }; std::vector tests({ {&fakeInfo, nullptr} }); - Catch::defaultListTests(sstream, tests, false, Catch::Verbosity::Normal); + Catch::ConfigData cd; + cd.useColour = Catch::UseColour::No; + Catch::Config conf(cd); + auto colour = Catch::makeColourImpl( &conf, &sstream); + Catch::defaultListTests(sstream.stream(), colour.get(), tests, false, Catch::Verbosity::Normal); auto listingString = sstream.str(); REQUIRE_THAT( listingString,