Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle ANSI escape sequences when performing column wrapping #2849

Merged
merged 13 commits into from May 4, 2024

Conversation

jeremy-rifkin
Copy link
Contributor

@jeremy-rifkin jeremy-rifkin commented Mar 29, 2024

Description

This PR adds functionality to skip around ANSI escape sequences in catch_textflow so they do not contribute to line length and line wrapping code does not split escape sequences in the middle. I've implemented this by creating a AnsiSkippingString abstraction that has a bidirectional iterator that can skip around escape sequences while iterating. Additionally I refactored Column::const_iterator to be iterator-based rather than index-based so this abstraction is a simple drop-in for std::string.

Currently only color sequences are handled, other escape sequences are left unaffected.

Motivation: Text with ANSI color sequences gets messed up when being output by Catch2 #2833.

For example, the current behavior of string with lots of escapes:
image
The example I have handy is a syntax-highlighted assertion from another library.

Behavior with this PR:
image

GitHub Issues

Closes #2833

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 98.69281% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 91.10%. Comparing base (bfe3ff8) to head (8f91452).
Report is 30 commits behind head on devel.

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #2849      +/-   ##
==========================================
- Coverage   91.17%   91.10%   -0.07%     
==========================================
  Files         197      198       +1     
  Lines        8379     8493     +114     
==========================================
+ Hits         7639     7737      +98     
- Misses        740      756      +16     

std::string const& current_line = m_column.m_string;
if ( m_lineStart < current_line.size() && current_line[m_lineStart] == '\n' ) {
m_lineStart += 1;
m_lineStart = m_lineEnd; // FIXME
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the fixme here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is resolved now and I just forgot to remove the comment

// constant value" warning on MSVC
static constexpr char sentinel = static_cast<char>( 0xffu );

AnsiSkippingString( std::string const& text );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a move overload as well.

}
}

std::string Column::const_iterator::operator*() const {
assert( m_lineStart <= m_parsedTo );
return addIndentAndSuffix( m_lineStart, m_lineLength );
// assert( m_lineStart <= m_parsedTo );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be commented out.

@jeremy-rifkin
Copy link
Contributor Author

Thank you for reviewing, I've updated to address the review comments

@horenmar horenmar merged commit 8ce2426 into catchorg:devel May 4, 2024
81 checks passed
@horenmar
Copy link
Member

horenmar commented May 4, 2024

thanks

@jeremy-rifkin
Copy link
Contributor Author

Thank you so much!

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

Successfully merging this pull request may close these issues.

Handle ANSI escape sequences during text wrapping
2 participants