From 90f366007ec33ec4137fdce2a608fe00664c02e5 Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Thu, 28 Mar 2024 20:45:10 -0500 Subject: [PATCH 01/13] Refactor to use iterators --- src/catch2/internal/catch_textflow.cpp | 45 +++++++++++++------------- src/catch2/internal/catch_textflow.hpp | 12 +++---- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index 857fd2b9f4..cd5560d723 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -46,60 +46,60 @@ namespace Catch { m_parsedTo = m_lineStart; std::string const& current_line = m_column.m_string; - if ( current_line[m_lineStart] == '\n' ) { + if ( *m_lineStart == '\n' ) { ++m_parsedTo; } const auto maxLineLength = m_column.m_width - indentSize(); - const auto maxParseTo = std::min(current_line.size(), m_lineStart + maxLineLength); - while ( m_parsedTo < maxParseTo && - current_line[m_parsedTo] != '\n' ) { + std::size_t parsed = 0; + while ( m_parsedTo != current_line.end() && parsed < maxLineLength && *m_parsedTo != '\n' ) { ++m_parsedTo; + ++parsed; } // If we encountered a newline before the column is filled, // then we linebreak at the newline and consider this line // finished. if ( m_parsedTo < m_lineStart + maxLineLength ) { - m_lineLength = m_parsedTo - m_lineStart; + m_lineEnd = m_parsedTo; } else { // Look for a natural linebreak boundary in the column // (We look from the end, so that the first found boundary is // the right one) size_t newLineLength = maxLineLength; - while ( newLineLength > 0 && !isBoundary( current_line, m_lineStart + newLineLength ) ) { + while ( newLineLength > 0 && !isBoundary( current_line, (m_lineStart - current_line.begin()) + newLineLength ) ) { --newLineLength; } while ( newLineLength > 0 && - isWhitespace( current_line[m_lineStart + newLineLength - 1] ) ) { + isWhitespace( *(m_lineStart + newLineLength - 1) ) ) { --newLineLength; } // If we found one, then that is where we linebreak if ( newLineLength > 0 ) { - m_lineLength = newLineLength; + m_lineEnd = m_lineStart + newLineLength; } else { // Otherwise we have to split text with a hyphen m_addHyphen = true; - m_lineLength = maxLineLength - 1; + m_lineEnd = m_lineStart + maxLineLength - 1; } } } size_t Column::const_iterator::indentSize() const { auto initial = - m_lineStart == 0 ? m_column.m_initialIndent : std::string::npos; + m_lineStart == m_column.m_string.begin() ? m_column.m_initialIndent : std::string::npos; return initial == std::string::npos ? m_column.m_indent : initial; } std::string - Column::const_iterator::addIndentAndSuffix( size_t position, - size_t length ) const { + Column::const_iterator::addIndentAndSuffix( std::string::const_iterator start, + std::string::const_iterator end ) const { std::string ret; const auto desired_indent = indentSize(); - ret.reserve( desired_indent + length + m_addHyphen ); + ret.reserve( desired_indent + (end - start) + m_addHyphen ); ret.append( desired_indent, ' ' ); - ret.append( m_column.m_string, position, length ); + ret.append( start, end ); if ( m_addHyphen ) { ret.push_back( '-' ); } @@ -107,34 +107,33 @@ namespace Catch { return ret; } - Column::const_iterator::const_iterator( Column const& column ): m_column( column ) { + Column::const_iterator::const_iterator( Column const& column ): m_column( column ), m_lineStart(column.m_string.begin()), m_lineEnd(column.m_string.begin()) { assert( m_column.m_width > m_column.m_indent ); assert( m_column.m_initialIndent == std::string::npos || m_column.m_width > m_column.m_initialIndent ); calcLength(); - if ( m_lineLength == 0 ) { - m_lineStart = m_column.m_string.size(); + if ( m_lineStart == m_lineEnd ) { + m_lineStart = m_column.m_string.end(); } } std::string Column::const_iterator::operator*() const { assert( m_lineStart <= m_parsedTo ); - return addIndentAndSuffix( m_lineStart, m_lineLength ); + return addIndentAndSuffix( m_lineStart, m_lineEnd ); } Column::const_iterator& Column::const_iterator::operator++() { - m_lineStart += m_lineLength; + m_lineStart = m_lineEnd; // FIXME std::string const& current_line = m_column.m_string; - if ( m_lineStart < current_line.size() && current_line[m_lineStart] == '\n' ) { + if ( m_lineStart < current_line.end() && *m_lineStart == '\n' ) { m_lineStart += 1; } else { - while ( m_lineStart < current_line.size() && - isWhitespace( current_line[m_lineStart] ) ) { + while ( m_lineStart < current_line.end() && isWhitespace( *m_lineStart ) ) { ++m_lineStart; } } - if ( m_lineStart != current_line.size() ) { + if ( m_lineStart != current_line.end() ) { calcLength(); } return *this; diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index a78451d559..58d6fc3f93 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -47,16 +47,16 @@ namespace Catch { Column const& m_column; // Where does the current line start? - size_t m_lineStart = 0; + std::string::const_iterator m_lineStart; // How long should the current line be? - size_t m_lineLength = 0; + std::string::const_iterator m_lineEnd; // How far have we checked the string to iterate? - size_t m_parsedTo = 0; + std::string::const_iterator m_parsedTo; // Should a '-' be appended to the line? bool m_addHyphen = false; const_iterator( Column const& column, EndTag ): - m_column( column ), m_lineStart( m_column.m_string.size() ) {} + m_column( column ), m_lineStart( m_column.m_string.end() ), m_lineEnd(column.m_string.end()) {} // Calculates the length of the current line void calcLength(); @@ -66,8 +66,8 @@ namespace Catch { // Creates an indented and (optionally) suffixed string from // current iterator position, indentation and length. - std::string addIndentAndSuffix( size_t position, - size_t length ) const; + std::string addIndentAndSuffix( std::string::const_iterator start, + std::string::const_iterator end ) const; public: using difference_type = std::ptrdiff_t; From 9cd4828093f668356b998c3704fcae8c7ee4fd68 Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Thu, 28 Mar 2024 21:06:01 -0500 Subject: [PATCH 02/13] More refactoring and simplification --- src/catch2/internal/catch_textflow.cpp | 39 ++++++++++++-------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index cd5560d723..b239485881 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -26,14 +26,11 @@ namespace { return std::memchr( chars, c, sizeof( chars ) - 1 ) != nullptr; } - bool isBoundary( std::string const& line, size_t at ) { - assert( at > 0 ); - assert( at <= line.size() ); - - return at == line.size() || - ( isWhitespace( line[at] ) && !isWhitespace( line[at - 1] ) ) || - isBreakableBefore( line[at] ) || - isBreakableAfter( line[at - 1] ); + bool isBoundary( std::string const& line, std::string::const_iterator it ) { + return it == line.end() || + ( isWhitespace( *it ) && !isWhitespace( *(it - 1) ) ) || + isBreakableBefore( *it ) || + isBreakableAfter( *(it - 1) ); } } // namespace @@ -51,37 +48,37 @@ namespace Catch { } const auto maxLineLength = m_column.m_width - indentSize(); - std::size_t parsed = 0; - while ( m_parsedTo != current_line.end() && parsed < maxLineLength && *m_parsedTo != '\n' ) { + std::size_t lineLength = 0; + while ( m_parsedTo != current_line.end() && lineLength < maxLineLength && *m_parsedTo != '\n' ) { ++m_parsedTo; - ++parsed; + ++lineLength; } // If we encountered a newline before the column is filled, // then we linebreak at the newline and consider this line // finished. - if ( m_parsedTo < m_lineStart + maxLineLength ) { + if ( lineLength < maxLineLength ) { m_lineEnd = m_parsedTo; } else { // Look for a natural linebreak boundary in the column // (We look from the end, so that the first found boundary is // the right one) - size_t newLineLength = maxLineLength; - while ( newLineLength > 0 && !isBoundary( current_line, (m_lineStart - current_line.begin()) + newLineLength ) ) { - --newLineLength; + m_lineEnd = m_parsedTo; + while ( lineLength > 0 && !isBoundary( current_line, m_lineEnd )) { + --lineLength; + --m_lineEnd; } - while ( newLineLength > 0 && - isWhitespace( *(m_lineStart + newLineLength - 1) ) ) { - --newLineLength; + while ( lineLength > 0 && isWhitespace( *(m_lineEnd - 1) ) ) { + --lineLength; + --m_lineEnd; } // If we found one, then that is where we linebreak - if ( newLineLength > 0 ) { - m_lineEnd = m_lineStart + newLineLength; + if ( lineLength > 0 ) { } else { // Otherwise we have to split text with a hyphen m_addHyphen = true; - m_lineEnd = m_lineStart + maxLineLength - 1; + m_lineEnd = m_parsedTo - 1; } } } From 096c74284e4521d2e289e371ea9a3014ef072bcc Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Thu, 28 Mar 2024 23:18:15 -0500 Subject: [PATCH 03/13] First pass --- src/catch2/internal/catch_textflow.cpp | 40 +++++----- src/catch2/internal/catch_textflow.hpp | 106 +++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 26 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index b239485881..c9f8525403 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -26,23 +26,25 @@ namespace { return std::memchr( chars, c, sizeof( chars ) - 1 ) != nullptr; } - bool isBoundary( std::string const& line, std::string::const_iterator it ) { - return it == line.end() || - ( isWhitespace( *it ) && !isWhitespace( *(it - 1) ) ) || - isBreakableBefore( *it ) || - isBreakableAfter( *(it - 1) ); - } - } // namespace namespace Catch { namespace TextFlow { + AnsiSkippingString::const_iterator AnsiSkippingString::begin() const { return const_iterator(m_string); } + AnsiSkippingString::const_iterator AnsiSkippingString::end() const { return const_iterator(m_string, const_iterator::EndTag{}); } + + static bool isBoundary( AnsiSkippingString const& line, AnsiSkippingString::const_iterator it ) { + return it == line.end() || + ( isWhitespace( *it ) && !isWhitespace( *it.one_before() ) ) || + isBreakableBefore( *it ) || + isBreakableAfter( *it.one_before() ); + } void Column::const_iterator::calcLength() { m_addHyphen = false; m_parsedTo = m_lineStart; - std::string const& current_line = m_column.m_string; + AnsiSkippingString const& current_line = m_column.m_string; if ( *m_lineStart == '\n' ) { ++m_parsedTo; } @@ -68,7 +70,7 @@ namespace Catch { --lineLength; --m_lineEnd; } - while ( lineLength > 0 && isWhitespace( *(m_lineEnd - 1) ) ) { + while ( lineLength > 0 && isWhitespace( *m_lineEnd.one_before() ) ) { --lineLength; --m_lineEnd; } @@ -78,7 +80,7 @@ namespace Catch { } else { // Otherwise we have to split text with a hyphen m_addHyphen = true; - m_lineEnd = m_parsedTo - 1; + m_lineEnd = m_parsedTo.one_before(); } } } @@ -90,11 +92,11 @@ namespace Catch { } std::string - Column::const_iterator::addIndentAndSuffix( std::string::const_iterator start, - std::string::const_iterator end ) const { + Column::const_iterator::addIndentAndSuffix( AnsiSkippingString::const_iterator start, + AnsiSkippingString::const_iterator end ) const { std::string ret; const auto desired_indent = indentSize(); - ret.reserve( desired_indent + (end - start) + m_addHyphen ); + // ret.reserve( desired_indent + (end - start) + m_addHyphen ); ret.append( desired_indent, ' ' ); ret.append( start, end ); if ( m_addHyphen ) { @@ -104,7 +106,7 @@ namespace Catch { return ret; } - Column::const_iterator::const_iterator( Column const& column ): m_column( column ), m_lineStart(column.m_string.begin()), m_lineEnd(column.m_string.begin()) { + Column::const_iterator::const_iterator( Column const& column ): m_column( column ), m_lineStart(column.m_string.begin()), m_lineEnd(column.m_string.begin()), m_parsedTo(column.m_string.begin()) { assert( m_column.m_width > m_column.m_indent ); assert( m_column.m_initialIndent == std::string::npos || m_column.m_width > m_column.m_initialIndent ); @@ -115,17 +117,17 @@ namespace Catch { } std::string Column::const_iterator::operator*() const { - assert( m_lineStart <= m_parsedTo ); + // assert( m_lineStart <= m_parsedTo ); return addIndentAndSuffix( m_lineStart, m_lineEnd ); } Column::const_iterator& Column::const_iterator::operator++() { m_lineStart = m_lineEnd; // FIXME - std::string const& current_line = m_column.m_string; - if ( m_lineStart < current_line.end() && *m_lineStart == '\n' ) { - m_lineStart += 1; + AnsiSkippingString const& current_line = m_column.m_string; + if ( m_lineStart != current_line.end() && *m_lineStart == '\n' ) { + m_lineStart++; } else { - while ( m_lineStart < current_line.end() && isWhitespace( *m_lineStart ) ) { + while ( m_lineStart != current_line.end() && isWhitespace( *m_lineStart ) ) { ++m_lineStart; } } diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index 58d6fc3f93..ed6286c395 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -20,6 +20,98 @@ namespace Catch { class Columns; + /** + * Abstraction for a string with ansi escape sequences that automatically skips over escapes when iterating. + * Only graphical escape sequences are considered. + * + * Internal representation: + * An escape sequence looks like \033[39;49m + * We need bidirectional iteration and tne unbound length of escape sequences poses a problem for operator-- + * To make this work we'll replace the last `m` with a 0xff (this is a codepoint that won't have any utf-8 + * meaning). + */ + class AnsiSkippingString { + std::string m_string; + std::size_t m_length; + public: + class const_iterator; + using iterator = const_iterator; + + AnsiSkippingString(std::string const& text) : m_string(text) {} + + const_iterator begin() const; + const_iterator end() const; + + size_t size() const { + return m_string.size(); + } + + std::string substring(std::size_t pos, std::size_t length) const { + return m_string.substr(pos, length); + } + }; + + class AnsiSkippingString::const_iterator { + friend AnsiSkippingString; + struct EndTag {}; + + const std::string* m_string; + std::string::const_iterator m_it; + + explicit const_iterator( const std::string& string, EndTag ): + m_string(&string), m_it(string.end()) {} + + void advance() { + // TODO + m_it++; + } + + public: + using difference_type = std::ptrdiff_t; + using value_type = char; + using pointer = value_type*; + using reference = value_type&; + using iterator_category = std::bidirectional_iterator_tag; + + // TODO: Advance if first character is ansi sequence + explicit const_iterator( const std::string& string ): m_string(&string), m_it(string.begin()) {} + + char operator*() const { + return *m_it; + } + + const_iterator& operator++() { + advance(); + return *this; + } + const_iterator operator++( int ) { + iterator prev( *this ); + operator++(); + return prev; + } + const_iterator& operator--() { + m_it--; + return *this; + } + const_iterator operator--( int ) { + iterator prev( *this ); + operator--(); + return prev; + } + + bool operator==( const_iterator const& other ) const { + return m_it == other.m_it; + } + bool operator!=( const_iterator const& other ) const { + return !operator==( other ); + } + + const_iterator one_before() const { + auto it = *this; + return --it; + } + }; + /** * Represents a column of text with specific width and indentation * @@ -29,7 +121,7 @@ namespace Catch { */ class Column { // String to be written out - std::string m_string; + AnsiSkippingString m_string; // Width of the column for linebreaking size_t m_width = CATCH_CONFIG_CONSOLE_WIDTH - 1; // Indentation of other lines (including first if initial indent is unset) @@ -47,16 +139,16 @@ namespace Catch { Column const& m_column; // Where does the current line start? - std::string::const_iterator m_lineStart; + AnsiSkippingString::const_iterator m_lineStart; // How long should the current line be? - std::string::const_iterator m_lineEnd; + AnsiSkippingString::const_iterator m_lineEnd; // How far have we checked the string to iterate? - std::string::const_iterator m_parsedTo; + AnsiSkippingString::const_iterator m_parsedTo; // Should a '-' be appended to the line? bool m_addHyphen = false; const_iterator( Column const& column, EndTag ): - m_column( column ), m_lineStart( m_column.m_string.end() ), m_lineEnd(column.m_string.end()) {} + m_column( column ), m_lineStart( m_column.m_string.end() ), m_lineEnd(column.m_string.end()), m_parsedTo(column.m_string.end()) {} // Calculates the length of the current line void calcLength(); @@ -66,8 +158,8 @@ namespace Catch { // Creates an indented and (optionally) suffixed string from // current iterator position, indentation and length. - std::string addIndentAndSuffix( std::string::const_iterator start, - std::string::const_iterator end ) const; + std::string addIndentAndSuffix( AnsiSkippingString::const_iterator start, + AnsiSkippingString::const_iterator end ) const; public: using difference_type = std::ptrdiff_t; From d69651a81ab5132577a5a7712aba3f3276f9d787 Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Fri, 29 Mar 2024 15:06:37 -0500 Subject: [PATCH 04/13] Implement escape sequence skipping --- src/catch2/internal/catch_textflow.cpp | 90 +++++++- src/catch2/internal/catch_textflow.hpp | 31 +-- .../IntrospectiveTests/TextFlow.tests.cpp | 194 ++++++++++++++++++ 3 files changed, 296 insertions(+), 19 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index c9f8525403..d21aec45f6 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -30,14 +30,93 @@ namespace { namespace Catch { namespace TextFlow { + void AnsiSkippingString::preprocessString() { + for(auto it = m_string.begin(); it != m_string.end(); ) { + // try to read through an ansi sequence + while(*it == '\033' && it + 1 != m_string.end() && *(it + 1) == '[') { + auto cursor = it + 2; + while(cursor != m_string.end() && (isdigit(*cursor) || *cursor == ';')) { + ++cursor; + } + if(cursor != m_string.end() && *cursor == 'm') { + // 'm' -> 0xff + *cursor = AnsiSkippingString::sentinel; + // if we've read an ansi sequence, set the iterator and return to the top of the loop + it = cursor + 1; + } else { + break; + } + } + if(it != m_string.end()) { + ++m_size; + ++it; + } + } + } + + AnsiSkippingString::AnsiSkippingString(std::string const& text) : m_string(text) { + preprocessString(); + } + AnsiSkippingString::const_iterator AnsiSkippingString::begin() const { return const_iterator(m_string); } + AnsiSkippingString::const_iterator AnsiSkippingString::end() const { return const_iterator(m_string, const_iterator::EndTag{}); } + std::string AnsiSkippingString::substring(const_iterator begin, const_iterator end) const { + // There's one caveat here to an otherwise simple substring: when making a begin iterator we might have + // skipped ansi sequences at the start. If `begin` here is a begin iterator, skipped over initial ansi + // sequences, we'll use the true beginning of the string. + // Lastly: We need to transform any chars we replaced with 0xff back to 'm' + auto str = std::string(begin == this->begin() ? m_string.begin() : begin.m_it, end.m_it); + std::transform(str.begin(), str.end(), str.begin(), [](char c) { + return c == AnsiSkippingString::sentinel ? 'm' : c; + }); + return str; + } + + void AnsiSkippingString::const_iterator::tryParseAnsiEscapes() { + // check if we've landed on an ansi sequence, and if so read through it + while(m_it != m_string->end() && *m_it == '\033' && m_it + 1 != m_string->end() && *(m_it + 1) == '[') { + auto cursor = m_it + 2; + while(cursor != m_string->end() && (isdigit(*cursor) || *cursor == ';')) { + ++cursor; + } + if(cursor != m_string->end() && *cursor == AnsiSkippingString::sentinel) { + // if we've read an ansi sequence, set the iterator and return to the top of the loop + m_it = cursor + 1; + } else { + break; + } + } + } + + void AnsiSkippingString::const_iterator::advance() { + m_it++; + tryParseAnsiEscapes(); + } + + void AnsiSkippingString::const_iterator::unadvance() { + assert(m_it != m_string->begin()); + m_it--; + // if *m_it is 0xff, scan back to the \033 and then m_it-- once more (and repeat check) + while(*m_it == AnsiSkippingString::sentinel) { + while(*m_it != '\033') { + assert(m_it != m_string->begin()); + m_it--; + } + // if this happens, we must have been a begin iterator that had skipped over ansi sequences at the + // start of a string + assert(m_it != m_string->begin()); + assert(*m_it == '\033'); + m_it--; + } + } + static bool isBoundary( AnsiSkippingString const& line, AnsiSkippingString::const_iterator it ) { return it == line.end() || - ( isWhitespace( *it ) && !isWhitespace( *it.one_before() ) ) || + ( isWhitespace( *it ) && !isWhitespace( *it.oneBefore() ) ) || isBreakableBefore( *it ) || - isBreakableAfter( *it.one_before() ); + isBreakableAfter( *it.oneBefore() ); } void Column::const_iterator::calcLength() { @@ -70,7 +149,7 @@ namespace Catch { --lineLength; --m_lineEnd; } - while ( lineLength > 0 && isWhitespace( *m_lineEnd.one_before() ) ) { + while ( lineLength > 0 && isWhitespace( *m_lineEnd.oneBefore() ) ) { --lineLength; --m_lineEnd; } @@ -80,7 +159,7 @@ namespace Catch { } else { // Otherwise we have to split text with a hyphen m_addHyphen = true; - m_lineEnd = m_parsedTo.one_before(); + m_lineEnd = m_parsedTo.oneBefore(); } } } @@ -98,7 +177,8 @@ namespace Catch { const auto desired_indent = indentSize(); // ret.reserve( desired_indent + (end - start) + m_addHyphen ); ret.append( desired_indent, ' ' ); - ret.append( start, end ); + // ret.append( start, end ); + ret += m_column.m_string.substring(start, end); if ( m_addHyphen ) { ret.push_back( '-' ); } diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index ed6286c395..90036de17d 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -32,23 +32,26 @@ namespace Catch { */ class AnsiSkippingString { std::string m_string; - std::size_t m_length; + std::size_t m_size = 0; + + // perform 0xff replacement and calculate m_size + void preprocessString(); + public: class const_iterator; using iterator = const_iterator; + static constexpr char sentinel = static_cast(0xff); - AnsiSkippingString(std::string const& text) : m_string(text) {} + AnsiSkippingString(std::string const& text); const_iterator begin() const; const_iterator end() const; size_t size() const { - return m_string.size(); + return m_size; } - std::string substring(std::size_t pos, std::size_t length) const { - return m_string.substr(pos, length); - } + std::string substring(const_iterator begin, const_iterator end) const; }; class AnsiSkippingString::const_iterator { @@ -61,10 +64,9 @@ namespace Catch { explicit const_iterator( const std::string& string, EndTag ): m_string(&string), m_it(string.end()) {} - void advance() { - // TODO - m_it++; - } + void tryParseAnsiEscapes(); + void advance(); + void unadvance(); public: using difference_type = std::ptrdiff_t; @@ -73,8 +75,9 @@ namespace Catch { using reference = value_type&; using iterator_category = std::bidirectional_iterator_tag; - // TODO: Advance if first character is ansi sequence - explicit const_iterator( const std::string& string ): m_string(&string), m_it(string.begin()) {} + explicit const_iterator( const std::string& string ): m_string(&string), m_it(string.begin()) { + tryParseAnsiEscapes(); + } char operator*() const { return *m_it; @@ -90,7 +93,7 @@ namespace Catch { return prev; } const_iterator& operator--() { - m_it--; + unadvance(); return *this; } const_iterator operator--( int ) { @@ -106,7 +109,7 @@ namespace Catch { return !operator==( other ); } - const_iterator one_before() const { + const_iterator oneBefore() const { auto it = *this; return --it; } diff --git a/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp b/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp index 653f65ba4c..893b2e10da 100644 --- a/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp @@ -12,6 +12,7 @@ #include using Catch::TextFlow::Column; +using Catch::TextFlow::AnsiSkippingString; namespace { static std::string as_written(Column const& c) { @@ -198,3 +199,196 @@ TEST_CASE( "#1400 - TextFlow::Column wrapping would sometimes duplicate words", " in \n" " convallis posuere, libero nisi ultricies orci, nec lobortis."); } + +TEST_CASE( "TextFlow::AnsiSkippingString skips ansi sequences", + "[TextFlow][ansiskippingstring][approvals]" ) { + + SECTION("basic string") { + std::string text = "abcde"; + AnsiSkippingString str(text); + + SECTION( "iterates forward" ) { + auto it = str.begin(); + CHECK(*it == 'a'); + ++it; + CHECK(*it == 'b'); + ++it; + CHECK(*it == 'c'); + ++it; + CHECK(*it == 'd'); + ++it; + CHECK(*it == 'e'); + ++it; + CHECK(it == str.end()); + } + SECTION( "iterates backwards" ) { + auto it = str.end(); + --it; + CHECK(*it == 'e'); + --it; + CHECK(*it == 'd'); + --it; + CHECK(*it == 'c'); + --it; + CHECK(*it == 'b'); + --it; + CHECK(*it == 'a'); + CHECK(it == str.begin()); + } + } + + SECTION( "ansi escape sequences at the start" ) { + std::string text = "abcde"; + AnsiSkippingString str(text); + auto it = str.begin(); + CHECK(*it == 'a'); + ++it; + CHECK(*it == 'b'); + ++it; + CHECK(*it == 'c'); + ++it; + CHECK(*it == 'd'); + ++it; + CHECK(*it == 'e'); + ++it; + CHECK(it == str.end()); + --it; + CHECK(*it == 'e'); + --it; + CHECK(*it == 'd'); + --it; + CHECK(*it == 'c'); + --it; + CHECK(*it == 'b'); + --it; + CHECK(*it == 'a'); + CHECK(it == str.begin()); + } + + SECTION( "ansi escape sequences at the end" ) { + std::string text = "abcde"; + AnsiSkippingString str(text); + auto it = str.begin(); + CHECK(*it == 'a'); + ++it; + CHECK(*it == 'b'); + ++it; + CHECK(*it == 'c'); + ++it; + CHECK(*it == 'd'); + ++it; + CHECK(*it == 'e'); + ++it; + CHECK(it == str.end()); + --it; + CHECK(*it == 'e'); + --it; + CHECK(*it == 'd'); + --it; + CHECK(*it == 'c'); + --it; + CHECK(*it == 'b'); + --it; + CHECK(*it == 'a'); + CHECK(it == str.begin()); + } + + SECTION( "skips consecutive escapes" ) { + std::string text = "abcde"; + AnsiSkippingString str(text); + auto it = str.begin(); + CHECK(*it == 'a'); + ++it; + CHECK(*it == 'b'); + ++it; + CHECK(*it == 'c'); + ++it; + CHECK(*it == 'd'); + ++it; + CHECK(*it == 'e'); + ++it; + CHECK(it == str.end()); + --it; + CHECK(*it == 'e'); + --it; + CHECK(*it == 'd'); + --it; + CHECK(*it == 'c'); + --it; + CHECK(*it == 'b'); + --it; + CHECK(*it == 'a'); + CHECK(it == str.begin()); + } +} + +TEST_CASE( "TextFlow::AnsiSkippingString computes the size properly", + "[TextFlow][ansiskippingstring][approvals]" ) { + std::string text = "abcde"; + AnsiSkippingString str(text); + CHECK(str.size() == 5); +} + +TEST_CASE( "TextFlow::AnsiSkippingString substrings properly", + "[TextFlow][ansiskippingstring][approvals]" ) { + SECTION("basic test") { + std::string text = "abcde"; + AnsiSkippingString str(text); + auto a = str.begin(); + auto b = str.begin(); + ++b; + ++b; + CHECK(str.substring(a, b) == "ab"); + ++a; + ++b; + CHECK(str.substring(a, b) == "bc"); + CHECK(str.substring(a, str.end()) == "bcde"); + CHECK(str.substring(str.begin(), str.end()) == text); + } + SECTION("escapes at the start") { + std::string text = "abcde"; + AnsiSkippingString str(text); + auto a = str.begin(); + auto b = str.begin(); + ++b; + ++b; + CHECK(str.substring(a, b) == "ab"); + ++a; + ++b; + CHECK(str.substring(a, b) == "bc"); + CHECK(str.substring(a, str.end()) == "bcde"); + CHECK(str.substring(str.begin(), str.end()) == text); + } + SECTION("escapes at the end") { + std::string text = "abcde"; + AnsiSkippingString str(text); + auto a = str.begin(); + auto b = str.begin(); + ++b; + ++b; + CHECK(str.substring(a, b) == "ab"); + ++a; + ++b; + CHECK(str.substring(a, b) == "bc"); + CHECK(str.substring(a, str.end()) == "bcde"); + CHECK(str.substring(str.begin(), str.end()) == text); + } +} + +TEST_CASE( "TextFlow::Column skips ansi escape sequences", + "[TextFlow][column][approvals]" ) { + std::string text = "The quick brown fox jumped over the lazy dog"; + Column col(text); + + SECTION( "width=20" ) { + col.width( 20 ); + REQUIRE( as_written( col ) == "The quick brown fox\n" + "jumped over the lazy\n" + "dog" ); + } + + SECTION( "width=80" ) { + col.width( 80 ); + REQUIRE( as_written( col ) == text ); + } +} From 44165c716f715572a0d342a98787e34cf86fd508 Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Fri, 29 Mar 2024 15:43:08 -0500 Subject: [PATCH 05/13] Fix typo --- src/catch2/internal/catch_textflow.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index 90036de17d..5229039078 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -26,7 +26,7 @@ namespace Catch { * * Internal representation: * An escape sequence looks like \033[39;49m - * We need bidirectional iteration and tne unbound length of escape sequences poses a problem for operator-- + * We need bidirectional iteration and the unbound length of escape sequences poses a problem for operator-- * To make this work we'll replace the last `m` with a 0xff (this is a codepoint that won't have any utf-8 * meaning). */ From bfaec5fabdacb75336103b5606835c3873d204ad Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Mon, 1 Apr 2024 23:18:39 -0500 Subject: [PATCH 06/13] Fixes for windows --- src/catch2/internal/catch_textflow.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index d21aec45f6..f4e2c3f593 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -33,7 +33,7 @@ namespace Catch { void AnsiSkippingString::preprocessString() { for(auto it = m_string.begin(); it != m_string.end(); ) { // try to read through an ansi sequence - while(*it == '\033' && it + 1 != m_string.end() && *(it + 1) == '[') { + while(it != m_string.end() && *it == '\033' && it + 1 != m_string.end() && *(it + 1) == '[') { auto cursor = it + 2; while(cursor != m_string.end() && (isdigit(*cursor) || *cursor == ';')) { ++cursor; @@ -122,8 +122,13 @@ namespace Catch { void Column::const_iterator::calcLength() { m_addHyphen = false; m_parsedTo = m_lineStart; - AnsiSkippingString const& current_line = m_column.m_string; + + if(m_parsedTo == current_line.end()) { + m_lineEnd = m_parsedTo; + return; + } + if ( *m_lineStart == '\n' ) { ++m_parsedTo; } @@ -154,10 +159,8 @@ namespace Catch { --m_lineEnd; } - // If we found one, then that is where we linebreak - if ( lineLength > 0 ) { - } else { - // Otherwise we have to split text with a hyphen + // If we found one, then that is where we linebreak, otherwise we have to split text with a hyphen + if ( lineLength == 0 ) { m_addHyphen = true; m_lineEnd = m_parsedTo.oneBefore(); } From 9ccf930ec445d9ccc08826ff777d299e04251fcb Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Tue, 2 Apr 2024 19:02:08 -0500 Subject: [PATCH 07/13] Make msvc happy --- src/catch2/internal/catch_textflow.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index 5229039078..b1d11d491c 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -40,7 +40,8 @@ namespace Catch { public: class const_iterator; using iterator = const_iterator; - static constexpr char sentinel = static_cast(0xff); + // note: must be u-suffixed or this will cause a "truncation of constant value" warning on MSVC + static constexpr char sentinel = static_cast(0xffu); AnsiSkippingString(std::string const& text); From 3819226358e971335f01673f4c1d841f10b4b1fa Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Tue, 2 Apr 2024 21:16:03 -0500 Subject: [PATCH 08/13] Clang-format --- src/catch2/internal/catch_textflow.cpp | 149 +++++++++++++++---------- src/catch2/internal/catch_textflow.hpp | 57 ++++++---- 2 files changed, 120 insertions(+), 86 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index f4e2c3f593..ca0030cfb7 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -31,58 +31,76 @@ namespace { namespace Catch { namespace TextFlow { void AnsiSkippingString::preprocessString() { - for(auto it = m_string.begin(); it != m_string.end(); ) { + for ( auto it = m_string.begin(); it != m_string.end(); ) { // try to read through an ansi sequence - while(it != m_string.end() && *it == '\033' && it + 1 != m_string.end() && *(it + 1) == '[') { + while ( it != m_string.end() && *it == '\033' && + it + 1 != m_string.end() && *( it + 1 ) == '[' ) { auto cursor = it + 2; - while(cursor != m_string.end() && (isdigit(*cursor) || *cursor == ';')) { + while ( cursor != m_string.end() && + ( isdigit( *cursor ) || *cursor == ';' ) ) { ++cursor; } - if(cursor != m_string.end() && *cursor == 'm') { + if ( cursor != m_string.end() && *cursor == 'm' ) { // 'm' -> 0xff *cursor = AnsiSkippingString::sentinel; - // if we've read an ansi sequence, set the iterator and return to the top of the loop + // if we've read an ansi sequence, set the iterator and + // return to the top of the loop it = cursor + 1; } else { break; } } - if(it != m_string.end()) { + if ( it != m_string.end() ) { ++m_size; ++it; } } } - AnsiSkippingString::AnsiSkippingString(std::string const& text) : m_string(text) { + AnsiSkippingString::AnsiSkippingString( std::string const& text ): + m_string( text ) { preprocessString(); } - AnsiSkippingString::const_iterator AnsiSkippingString::begin() const { return const_iterator(m_string); } + AnsiSkippingString::const_iterator AnsiSkippingString::begin() const { + return const_iterator( m_string ); + } - AnsiSkippingString::const_iterator AnsiSkippingString::end() const { return const_iterator(m_string, const_iterator::EndTag{}); } + AnsiSkippingString::const_iterator AnsiSkippingString::end() const { + return const_iterator( m_string, const_iterator::EndTag{} ); + } - std::string AnsiSkippingString::substring(const_iterator begin, const_iterator end) const { - // There's one caveat here to an otherwise simple substring: when making a begin iterator we might have - // skipped ansi sequences at the start. If `begin` here is a begin iterator, skipped over initial ansi - // sequences, we'll use the true beginning of the string. - // Lastly: We need to transform any chars we replaced with 0xff back to 'm' - auto str = std::string(begin == this->begin() ? m_string.begin() : begin.m_it, end.m_it); - std::transform(str.begin(), str.end(), str.begin(), [](char c) { + std::string AnsiSkippingString::substring( const_iterator begin, + const_iterator end ) const { + // There's one caveat here to an otherwise simple substring: when + // making a begin iterator we might have skipped ansi sequences at + // the start. If `begin` here is a begin iterator, skipped over + // initial ansi sequences, we'll use the true beginning of the + // string. Lastly: We need to transform any chars we replaced with + // 0xff back to 'm' + auto str = std::string( begin == this->begin() ? m_string.begin() + : begin.m_it, + end.m_it ); + std::transform( str.begin(), str.end(), str.begin(), []( char c ) { return c == AnsiSkippingString::sentinel ? 'm' : c; - }); + } ); return str; } void AnsiSkippingString::const_iterator::tryParseAnsiEscapes() { - // check if we've landed on an ansi sequence, and if so read through it - while(m_it != m_string->end() && *m_it == '\033' && m_it + 1 != m_string->end() && *(m_it + 1) == '[') { + // check if we've landed on an ansi sequence, and if so read through + // it + while ( m_it != m_string->end() && *m_it == '\033' && + m_it + 1 != m_string->end() && *( m_it + 1 ) == '[' ) { auto cursor = m_it + 2; - while(cursor != m_string->end() && (isdigit(*cursor) || *cursor == ';')) { + while ( cursor != m_string->end() && + ( isdigit( *cursor ) || *cursor == ';' ) ) { ++cursor; } - if(cursor != m_string->end() && *cursor == AnsiSkippingString::sentinel) { - // if we've read an ansi sequence, set the iterator and return to the top of the loop + if ( cursor != m_string->end() && + *cursor == AnsiSkippingString::sentinel ) { + // if we've read an ansi sequence, set the iterator and + // return to the top of the loop m_it = cursor + 1; } else { break; @@ -96,27 +114,30 @@ namespace Catch { } void AnsiSkippingString::const_iterator::unadvance() { - assert(m_it != m_string->begin()); + assert( m_it != m_string->begin() ); m_it--; - // if *m_it is 0xff, scan back to the \033 and then m_it-- once more (and repeat check) - while(*m_it == AnsiSkippingString::sentinel) { - while(*m_it != '\033') { - assert(m_it != m_string->begin()); + // if *m_it is 0xff, scan back to the \033 and then m_it-- once more + // (and repeat check) + while ( *m_it == AnsiSkippingString::sentinel ) { + while ( *m_it != '\033' ) { + assert( m_it != m_string->begin() ); m_it--; } - // if this happens, we must have been a begin iterator that had skipped over ansi sequences at the - // start of a string - assert(m_it != m_string->begin()); - assert(*m_it == '\033'); + // if this happens, we must have been a begin iterator that had + // skipped over ansi sequences at the start of a string + assert( m_it != m_string->begin() ); + assert( *m_it == '\033' ); m_it--; } } - static bool isBoundary( AnsiSkippingString const& line, AnsiSkippingString::const_iterator it ) { + static bool isBoundary( AnsiSkippingString const& line, + AnsiSkippingString::const_iterator it ) { return it == line.end() || - ( isWhitespace( *it ) && !isWhitespace( *it.oneBefore() ) ) || - isBreakableBefore( *it ) || - isBreakableAfter( *it.oneBefore() ); + ( isWhitespace( *it ) && + !isWhitespace( *it.oneBefore() ) ) || + isBreakableBefore( *it ) || + isBreakableAfter( *it.oneBefore() ); } void Column::const_iterator::calcLength() { @@ -124,18 +145,17 @@ namespace Catch { m_parsedTo = m_lineStart; AnsiSkippingString const& current_line = m_column.m_string; - if(m_parsedTo == current_line.end()) { + if ( m_parsedTo == current_line.end() ) { m_lineEnd = m_parsedTo; return; } - if ( *m_lineStart == '\n' ) { - ++m_parsedTo; - } + if ( *m_lineStart == '\n' ) { ++m_parsedTo; } const auto maxLineLength = m_column.m_width - indentSize(); std::size_t lineLength = 0; - while ( m_parsedTo != current_line.end() && lineLength < maxLineLength && *m_parsedTo != '\n' ) { + while ( m_parsedTo != current_line.end() && + lineLength < maxLineLength && *m_parsedTo != '\n' ) { ++m_parsedTo; ++lineLength; } @@ -150,16 +170,19 @@ namespace Catch { // (We look from the end, so that the first found boundary is // the right one) m_lineEnd = m_parsedTo; - while ( lineLength > 0 && !isBoundary( current_line, m_lineEnd )) { + while ( lineLength > 0 && + !isBoundary( current_line, m_lineEnd ) ) { --lineLength; --m_lineEnd; } - while ( lineLength > 0 && isWhitespace( *m_lineEnd.oneBefore() ) ) { + while ( lineLength > 0 && + isWhitespace( *m_lineEnd.oneBefore() ) ) { --lineLength; --m_lineEnd; } - // If we found one, then that is where we linebreak, otherwise we have to split text with a hyphen + // If we found one, then that is where we linebreak, otherwise + // we have to split text with a hyphen if ( lineLength == 0 ) { m_addHyphen = true; m_lineEnd = m_parsedTo.oneBefore(); @@ -168,28 +191,31 @@ namespace Catch { } size_t Column::const_iterator::indentSize() const { - auto initial = - m_lineStart == m_column.m_string.begin() ? m_column.m_initialIndent : std::string::npos; + auto initial = m_lineStart == m_column.m_string.begin() + ? m_column.m_initialIndent + : std::string::npos; return initial == std::string::npos ? m_column.m_indent : initial; } - std::string - Column::const_iterator::addIndentAndSuffix( AnsiSkippingString::const_iterator start, - AnsiSkippingString::const_iterator end ) const { + std::string Column::const_iterator::addIndentAndSuffix( + AnsiSkippingString::const_iterator start, + AnsiSkippingString::const_iterator end ) const { std::string ret; const auto desired_indent = indentSize(); // ret.reserve( desired_indent + (end - start) + m_addHyphen ); ret.append( desired_indent, ' ' ); // ret.append( start, end ); - ret += m_column.m_string.substring(start, end); - if ( m_addHyphen ) { - ret.push_back( '-' ); - } + ret += m_column.m_string.substring( start, end ); + if ( m_addHyphen ) { ret.push_back( '-' ); } return ret; } - Column::const_iterator::const_iterator( Column const& column ): m_column( column ), m_lineStart(column.m_string.begin()), m_lineEnd(column.m_string.begin()), m_parsedTo(column.m_string.begin()) { + Column::const_iterator::const_iterator( Column const& column ): + m_column( column ), + m_lineStart( column.m_string.begin() ), + m_lineEnd( column.m_string.begin() ), + m_parsedTo( column.m_string.begin() ) { assert( m_column.m_width > m_column.m_indent ); assert( m_column.m_initialIndent == std::string::npos || m_column.m_width > m_column.m_initialIndent ); @@ -210,14 +236,13 @@ namespace Catch { if ( m_lineStart != current_line.end() && *m_lineStart == '\n' ) { m_lineStart++; } else { - while ( m_lineStart != current_line.end() && isWhitespace( *m_lineStart ) ) { + while ( m_lineStart != current_line.end() && + isWhitespace( *m_lineStart ) ) { ++m_lineStart; } } - if ( m_lineStart != current_line.end() ) { - calcLength(); - } + if ( m_lineStart != current_line.end() ) { calcLength(); } return *this; } @@ -314,25 +339,25 @@ namespace Catch { return os; } - Columns operator+(Column const& lhs, Column const& rhs) { + Columns operator+( Column const& lhs, Column const& rhs ) { Columns cols; cols += lhs; cols += rhs; return cols; } - Columns operator+(Column&& lhs, Column&& rhs) { + Columns operator+( Column&& lhs, Column&& rhs ) { Columns cols; cols += CATCH_MOVE( lhs ); cols += CATCH_MOVE( rhs ); return cols; } - Columns& operator+=(Columns& lhs, Column const& rhs) { + Columns& operator+=( Columns& lhs, Column const& rhs ) { lhs.m_columns.push_back( rhs ); return lhs; } - Columns& operator+=(Columns& lhs, Column&& rhs) { - lhs.m_columns.push_back( CATCH_MOVE(rhs) ); + Columns& operator+=( Columns& lhs, Column&& rhs ) { + lhs.m_columns.push_back( CATCH_MOVE( rhs ) ); return lhs; } Columns operator+( Columns const& lhs, Column const& rhs ) { diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index b1d11d491c..8ff518ca21 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -21,14 +21,16 @@ namespace Catch { class Columns; /** - * Abstraction for a string with ansi escape sequences that automatically skips over escapes when iterating. - * Only graphical escape sequences are considered. + * Abstraction for a string with ansi escape sequences that + * automatically skips over escapes when iterating. Only graphical + * escape sequences are considered. * * Internal representation: * An escape sequence looks like \033[39;49m - * We need bidirectional iteration and the unbound length of escape sequences poses a problem for operator-- - * To make this work we'll replace the last `m` with a 0xff (this is a codepoint that won't have any utf-8 - * meaning). + * We need bidirectional iteration and the unbound length of escape + * sequences poses a problem for operator-- To make this work we'll + * replace the last `m` with a 0xff (this is a codepoint that won't have + * any utf-8 meaning). */ class AnsiSkippingString { std::string m_string; @@ -40,19 +42,19 @@ namespace Catch { public: class const_iterator; using iterator = const_iterator; - // note: must be u-suffixed or this will cause a "truncation of constant value" warning on MSVC - static constexpr char sentinel = static_cast(0xffu); + // note: must be u-suffixed or this will cause a "truncation of + // constant value" warning on MSVC + static constexpr char sentinel = static_cast( 0xffu ); - AnsiSkippingString(std::string const& text); + AnsiSkippingString( std::string const& text ); const_iterator begin() const; const_iterator end() const; - size_t size() const { - return m_size; - } + size_t size() const { return m_size; } - std::string substring(const_iterator begin, const_iterator end) const; + std::string substring( const_iterator begin, + const_iterator end ) const; }; class AnsiSkippingString::const_iterator { @@ -63,7 +65,7 @@ namespace Catch { std::string::const_iterator m_it; explicit const_iterator( const std::string& string, EndTag ): - m_string(&string), m_it(string.end()) {} + m_string( &string ), m_it( string.end() ) {} void tryParseAnsiEscapes(); void advance(); @@ -76,13 +78,12 @@ namespace Catch { using reference = value_type&; using iterator_category = std::bidirectional_iterator_tag; - explicit const_iterator( const std::string& string ): m_string(&string), m_it(string.begin()) { + explicit const_iterator( const std::string& string ): + m_string( &string ), m_it( string.begin() ) { tryParseAnsiEscapes(); } - char operator*() const { - return *m_it; - } + char operator*() const { return *m_it; } const_iterator& operator++() { advance(); @@ -128,7 +129,8 @@ namespace Catch { AnsiSkippingString m_string; // Width of the column for linebreaking size_t m_width = CATCH_CONFIG_CONSOLE_WIDTH - 1; - // Indentation of other lines (including first if initial indent is unset) + // Indentation of other lines (including first if initial indent is + // unset) size_t m_indent = 0; // Indentation of the first line size_t m_initialIndent = std::string::npos; @@ -152,7 +154,10 @@ namespace Catch { bool m_addHyphen = false; const_iterator( Column const& column, EndTag ): - m_column( column ), m_lineStart( m_column.m_string.end() ), m_lineEnd(column.m_string.end()), m_parsedTo(column.m_string.end()) {} + m_column( column ), + m_lineStart( m_column.m_string.end() ), + m_lineEnd( column.m_string.end() ), + m_parsedTo( column.m_string.end() ) {} // Calculates the length of the current line void calcLength(); @@ -162,8 +167,9 @@ namespace Catch { // Creates an indented and (optionally) suffixed string from // current iterator position, indentation and length. - std::string addIndentAndSuffix( AnsiSkippingString::const_iterator start, - AnsiSkippingString::const_iterator end ) const; + std::string addIndentAndSuffix( + AnsiSkippingString::const_iterator start, + AnsiSkippingString::const_iterator end ) const; public: using difference_type = std::ptrdiff_t; @@ -180,7 +186,8 @@ namespace Catch { const_iterator operator++( int ); bool operator==( const_iterator const& other ) const { - return m_lineStart == other.m_lineStart && &m_column == &other.m_column; + return m_lineStart == other.m_lineStart && + &m_column == &other.m_column; } bool operator!=( const_iterator const& other ) const { return !operator==( other ); @@ -190,7 +197,7 @@ namespace Catch { explicit Column( std::string const& text ): m_string( text ) {} explicit Column( std::string&& text ): - m_string( CATCH_MOVE(text)) {} + m_string( CATCH_MOVE( text ) ) {} Column& width( size_t newWidth ) & { assert( newWidth > 0 ); @@ -221,7 +228,9 @@ namespace Catch { size_t width() const { return m_width; } const_iterator begin() const { return const_iterator( *this ); } - const_iterator end() const { return { *this, const_iterator::EndTag{} }; } + const_iterator end() const { + return { *this, const_iterator::EndTag{} }; + } friend std::ostream& operator<<( std::ostream& os, Column const& col ); From dab58896a25f195b638135004b890d7d8ee32f2d Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Tue, 2 Apr 2024 21:22:43 -0500 Subject: [PATCH 09/13] Add a couple more assertions --- src/catch2/internal/catch_textflow.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index ca0030cfb7..548494b286 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -109,6 +109,7 @@ namespace Catch { } void AnsiSkippingString::const_iterator::advance() { + assert( m_it != m_string->end() ); m_it++; tryParseAnsiEscapes(); } @@ -150,6 +151,7 @@ namespace Catch { return; } + assert( m_lineStart != current_line.end() ); if ( *m_lineStart == '\n' ) { ++m_parsedTo; } const auto maxLineLength = m_column.m_width - indentSize(); From 7c586ce8588631a85dc9b936b4cf78a2d9e5f0aa Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Tue, 9 Apr 2024 18:10:28 -0500 Subject: [PATCH 10/13] Review comments --- src/catch2/internal/catch_textflow.cpp | 41 ++++++++++++++------------ src/catch2/internal/catch_textflow.hpp | 3 +- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index 548494b286..445cdb712c 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -33,22 +33,21 @@ namespace Catch { void AnsiSkippingString::preprocessString() { for ( auto it = m_string.begin(); it != m_string.end(); ) { // try to read through an ansi sequence - while ( it != m_string.end() && *it == '\033' && - it + 1 != m_string.end() && *( it + 1 ) == '[' ) { + while ( it + 1 < m_string.end() && *it == '\033' && + *( it + 1 ) == '[' ) { auto cursor = it + 2; while ( cursor != m_string.end() && ( isdigit( *cursor ) || *cursor == ';' ) ) { ++cursor; } - if ( cursor != m_string.end() && *cursor == 'm' ) { - // 'm' -> 0xff - *cursor = AnsiSkippingString::sentinel; - // if we've read an ansi sequence, set the iterator and - // return to the top of the loop - it = cursor + 1; - } else { + if ( cursor == m_string.end() || *cursor != 'm' ) { break; } + // 'm' -> 0xff + *cursor = AnsiSkippingString::sentinel; + // if we've read an ansi sequence, set the iterator and + // return to the top of the loop + it = cursor + 1; } if ( it != m_string.end() ) { ++m_size; @@ -62,6 +61,11 @@ namespace Catch { preprocessString(); } + AnsiSkippingString::AnsiSkippingString( std::string&& text ): + m_string( CATCH_MOVE( text ) ) { + preprocessString(); + } + AnsiSkippingString::const_iterator AnsiSkippingString::begin() const { return const_iterator( m_string ); } @@ -90,21 +94,20 @@ namespace Catch { void AnsiSkippingString::const_iterator::tryParseAnsiEscapes() { // check if we've landed on an ansi sequence, and if so read through // it - while ( m_it != m_string->end() && *m_it == '\033' && - m_it + 1 != m_string->end() && *( m_it + 1 ) == '[' ) { + while ( m_it + 1 < m_string->end() && *m_it == '\033' && + *( m_it + 1 ) == '[' ) { auto cursor = m_it + 2; while ( cursor != m_string->end() && ( isdigit( *cursor ) || *cursor == ';' ) ) { ++cursor; } - if ( cursor != m_string->end() && - *cursor == AnsiSkippingString::sentinel ) { - // if we've read an ansi sequence, set the iterator and - // return to the top of the loop - m_it = cursor + 1; - } else { + if ( cursor == m_string->end() || + *cursor != AnsiSkippingString::sentinel ) { break; } + // if we've read an ansi sequence, set the iterator and + // return to the top of the loop + m_it = cursor + 1; } } @@ -228,12 +231,12 @@ namespace Catch { } std::string Column::const_iterator::operator*() const { - // assert( m_lineStart <= m_parsedTo ); + assert( m_lineStart != m_parsedTo ); return addIndentAndSuffix( m_lineStart, m_lineEnd ); } Column::const_iterator& Column::const_iterator::operator++() { - m_lineStart = m_lineEnd; // FIXME + m_lineStart = m_lineEnd; AnsiSkippingString const& current_line = m_column.m_string; if ( m_lineStart != current_line.end() && *m_lineStart == '\n' ) { m_lineStart++; diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index 8ff518ca21..510b0516d7 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -46,7 +46,8 @@ namespace Catch { // constant value" warning on MSVC static constexpr char sentinel = static_cast( 0xffu ); - AnsiSkippingString( std::string const& text ); + explicit AnsiSkippingString( std::string const& text ); + explicit AnsiSkippingString( std::string&& text ); const_iterator begin() const; const_iterator end() const; From 4883193157208f16c42918c8ab423e78b358457f Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Tue, 9 Apr 2024 18:15:32 -0500 Subject: [PATCH 11/13] Add an operator<= to the ansi string iterator --- src/catch2/internal/catch_textflow.cpp | 2 +- src/catch2/internal/catch_textflow.hpp | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index 445cdb712c..cae8dd7148 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -231,7 +231,7 @@ namespace Catch { } std::string Column::const_iterator::operator*() const { - assert( m_lineStart != m_parsedTo ); + assert( m_lineStart <= m_parsedTo ); return addIndentAndSuffix( m_lineStart, m_lineEnd ); } diff --git a/src/catch2/internal/catch_textflow.hpp b/src/catch2/internal/catch_textflow.hpp index 510b0516d7..2d9d78a50a 100644 --- a/src/catch2/internal/catch_textflow.hpp +++ b/src/catch2/internal/catch_textflow.hpp @@ -111,6 +111,9 @@ namespace Catch { bool operator!=( const_iterator const& other ) const { return !operator==( other ); } + bool operator<=( const_iterator const& other ) const { + return m_it <= other.m_it; + } const_iterator oneBefore() const { auto it = *this; From 46cbfb2e6529447fe2bd2f61edc4b72eb53d8bb4 Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Tue, 9 Apr 2024 19:49:02 -0500 Subject: [PATCH 12/13] Revert change that resulted in seeking past end --- src/catch2/internal/catch_textflow.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/catch2/internal/catch_textflow.cpp b/src/catch2/internal/catch_textflow.cpp index cae8dd7148..1c21d20e56 100644 --- a/src/catch2/internal/catch_textflow.cpp +++ b/src/catch2/internal/catch_textflow.cpp @@ -33,8 +33,8 @@ namespace Catch { void AnsiSkippingString::preprocessString() { for ( auto it = m_string.begin(); it != m_string.end(); ) { // try to read through an ansi sequence - while ( it + 1 < m_string.end() && *it == '\033' && - *( it + 1 ) == '[' ) { + while ( it != m_string.end() && *it == '\033' && + it + 1 != m_string.end() && *( it + 1 ) == '[' ) { auto cursor = it + 2; while ( cursor != m_string.end() && ( isdigit( *cursor ) || *cursor == ';' ) ) { @@ -94,8 +94,8 @@ namespace Catch { void AnsiSkippingString::const_iterator::tryParseAnsiEscapes() { // check if we've landed on an ansi sequence, and if so read through // it - while ( m_it + 1 < m_string->end() && *m_it == '\033' && - *( m_it + 1 ) == '[' ) { + while ( m_it != m_string->end() && *m_it == '\033' && + m_it + 1 != m_string->end() && *( m_it + 1 ) == '[' ) { auto cursor = m_it + 2; while ( cursor != m_string->end() && ( isdigit( *cursor ) || *cursor == ';' ) ) { From 8f914528a9eaa0234c60d44573298d9af30c2e6e Mon Sep 17 00:00:00 2001 From: Jeremy <51220084+jeremy-rifkin@users.noreply.github.com> Date: Fri, 3 May 2024 23:04:51 -0500 Subject: [PATCH 13/13] Replace literal escape codes with \033 and add a test case for incomplete ansi escape sequences --- .../IntrospectiveTests/TextFlow.tests.cpp | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp b/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp index 893b2e10da..de03ed09af 100644 --- a/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/TextFlow.tests.cpp @@ -204,7 +204,7 @@ TEST_CASE( "TextFlow::AnsiSkippingString skips ansi sequences", "[TextFlow][ansiskippingstring][approvals]" ) { SECTION("basic string") { - std::string text = "abcde"; + std::string text = "a\033[38;2;98;174;239mb\033[38mc\033[0md\033[me"; AnsiSkippingString str(text); SECTION( "iterates forward" ) { @@ -238,7 +238,7 @@ TEST_CASE( "TextFlow::AnsiSkippingString skips ansi sequences", } SECTION( "ansi escape sequences at the start" ) { - std::string text = "abcde"; + std::string text = "\033[38;2;98;174;239ma\033[38;2;98;174;239mb\033[38mc\033[0md\033[me"; AnsiSkippingString str(text); auto it = str.begin(); CHECK(*it == 'a'); @@ -266,7 +266,7 @@ TEST_CASE( "TextFlow::AnsiSkippingString skips ansi sequences", } SECTION( "ansi escape sequences at the end" ) { - std::string text = "abcde"; + std::string text = "a\033[38;2;98;174;239mb\033[38mc\033[0md\033[me\033[38;2;98;174;239m"; AnsiSkippingString str(text); auto it = str.begin(); CHECK(*it == 'a'); @@ -294,7 +294,7 @@ TEST_CASE( "TextFlow::AnsiSkippingString skips ansi sequences", } SECTION( "skips consecutive escapes" ) { - std::string text = "abcde"; + std::string text = "\033[38;2;98;174;239m\033[38;2;98;174;239ma\033[38;2;98;174;239mb\033[38m\033[38m\033[38mc\033[0md\033[me"; AnsiSkippingString str(text); auto it = str.begin(); CHECK(*it == 'a'); @@ -320,11 +320,17 @@ TEST_CASE( "TextFlow::AnsiSkippingString skips ansi sequences", CHECK(*it == 'a'); CHECK(it == str.begin()); } + + SECTION( "handles incomplete ansi sequences" ) { + std::string text = "a\033[b\033[30c\033[30;d\033[30;2e"; + AnsiSkippingString str(text); + CHECK(std::string(str.begin(), str.end()) == text); + } } TEST_CASE( "TextFlow::AnsiSkippingString computes the size properly", "[TextFlow][ansiskippingstring][approvals]" ) { - std::string text = "abcde"; + std::string text = "\033[38;2;98;174;239m\033[38;2;98;174;239ma\033[38;2;98;174;239mb\033[38m\033[38m\033[38mc\033[0md\033[me"; AnsiSkippingString str(text); CHECK(str.size() == 5); } @@ -332,59 +338,59 @@ TEST_CASE( "TextFlow::AnsiSkippingString computes the size properly", TEST_CASE( "TextFlow::AnsiSkippingString substrings properly", "[TextFlow][ansiskippingstring][approvals]" ) { SECTION("basic test") { - std::string text = "abcde"; + std::string text = "a\033[38;2;98;174;239mb\033[38mc\033[0md\033[me"; AnsiSkippingString str(text); auto a = str.begin(); auto b = str.begin(); ++b; ++b; - CHECK(str.substring(a, b) == "ab"); + CHECK(str.substring(a, b) == "a\033[38;2;98;174;239mb\033[38m"); ++a; ++b; - CHECK(str.substring(a, b) == "bc"); - CHECK(str.substring(a, str.end()) == "bcde"); + CHECK(str.substring(a, b) == "b\033[38mc\033[0m"); + CHECK(str.substring(a, str.end()) == "b\033[38mc\033[0md\033[me"); CHECK(str.substring(str.begin(), str.end()) == text); } SECTION("escapes at the start") { - std::string text = "abcde"; + std::string text = "\033[38;2;98;174;239m\033[38;2;98;174;239ma\033[38;2;98;174;239mb\033[38m\033[38m\033[38mc\033[0md\033[me"; AnsiSkippingString str(text); auto a = str.begin(); auto b = str.begin(); ++b; ++b; - CHECK(str.substring(a, b) == "ab"); + CHECK(str.substring(a, b) == "\033[38;2;98;174;239m\033[38;2;98;174;239ma\033[38;2;98;174;239mb\033[38m\033[38m\033[38m"); ++a; ++b; - CHECK(str.substring(a, b) == "bc"); - CHECK(str.substring(a, str.end()) == "bcde"); + CHECK(str.substring(a, b) == "b\033[38m\033[38m\033[38mc\033[0m"); + CHECK(str.substring(a, str.end()) == "b\033[38m\033[38m\033[38mc\033[0md\033[me"); CHECK(str.substring(str.begin(), str.end()) == text); } SECTION("escapes at the end") { - std::string text = "abcde"; + std::string text = "a\033[38;2;98;174;239mb\033[38mc\033[0md\033[me\033[38m"; AnsiSkippingString str(text); auto a = str.begin(); auto b = str.begin(); ++b; ++b; - CHECK(str.substring(a, b) == "ab"); + CHECK(str.substring(a, b) == "a\033[38;2;98;174;239mb\033[38m"); ++a; ++b; - CHECK(str.substring(a, b) == "bc"); - CHECK(str.substring(a, str.end()) == "bcde"); + CHECK(str.substring(a, b) == "b\033[38mc\033[0m"); + CHECK(str.substring(a, str.end()) == "b\033[38mc\033[0md\033[me\033[38m"); CHECK(str.substring(str.begin(), str.end()) == text); } } TEST_CASE( "TextFlow::Column skips ansi escape sequences", "[TextFlow][column][approvals]" ) { - std::string text = "The quick brown fox jumped over the lazy dog"; + std::string text = "\033[38;2;98;174;239m\033[38;2;198;120;221mThe quick brown \033[38;2;198;120;221mfox jumped over the lazy dog\033[0m"; Column col(text); SECTION( "width=20" ) { col.width( 20 ); - REQUIRE( as_written( col ) == "The quick brown fox\n" + REQUIRE( as_written( col ) == "\033[38;2;98;174;239m\033[38;2;198;120;221mThe quick brown \033[38;2;198;120;221mfox\n" "jumped over the lazy\n" - "dog" ); + "dog\033[0m" ); } SECTION( "width=80" ) {