Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use stringbuf instead of ostringstream
  • Loading branch information
ianthomas23 committed Oct 10, 2025
commit 44c04bea613db627816467e28e0060a082deb53a
4 changes: 2 additions & 2 deletions src/utils/terminal_pager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ void terminal_pager::maybe_grab_cout()
if (!m_grabbed && termcolor::_internal::is_atty(std::cout))
{
// Should we do anything with cerr?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should probably capture std::cerr too as by default (at least on some platforms) it outputs to the same place as std::cout

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We do need a strategy for what to do with cerr, but I don't know what is best yet.

Currently cerr will be written to the normal terminal buffer, so it won't be seen whilst the alternative buffer is being used but it reappears when the alternative buffer is disabled. This is perhaps not a good solution, but I don't think it is too bad for a first implementation. Some other options:

  1. Capture it and sent it to the same place as cout, it will appear in the output flow in the alternative buffer. We'd probably need to colour it to make is easily visible as it could be anywhere.
  2. Capture it and display it at the bottom of the alternative buffer for better visibility, but then there might be quite a lot of information displayed.
  3. Capture it and if anything at all is written to it we could avoid using the pager at all, just display the errors in the normal terminal flow.

Copy link
Copy Markdown
Member

@JohanMabille JohanMabille Oct 17, 2025

Choose a reason for hiding this comment

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

I have a preference for option 1, but I think we can discuss it in a dedicated issue and solve it in a dedicated PR so that it does not block this one, WDYT?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I agree let's deal with cerr separately so that we can merge and use this now.

m_cout_rdbuf = std::cout.rdbuf(m_oss.rdbuf());
m_cout_rdbuf = std::cout.rdbuf(&m_stringbuf);
m_grabbed = true;
}
}
Expand Down Expand Up @@ -173,7 +173,7 @@ void terminal_pager::show()

release_cout();

split_input_at_newlines(m_oss.view());
split_input_at_newlines(m_stringbuf.view());

update_terminal_size();
if (m_rows == 0 || m_lines.size() <= m_rows - 1)
Expand Down
2 changes: 1 addition & 1 deletion src/utils/terminal_pager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class terminal_pager


bool m_grabbed;
std::ostringstream m_oss;
std::stringbuf m_stringbuf;
std::streambuf* m_cout_rdbuf;
std::vector<std::string> m_lines;
size_t m_rows, m_columns;
Expand Down