Skip to content

Implement terminal_pager for log subcommand #46

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Implement terminal_pager for log subcommand #46

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
97cf63b
Implement terminal_pager for log subcommand
ianthomas23 Sep 19, 2025
e083e48
Include cstdint
ianthomas23 Sep 22, 2025
44c04be
Use stringbuf instead of ostringstream
ianthomas23 Oct 10, 2025
f7e4643
Remove m_grabbed
ianthomas23 Oct 10, 2025
0edd017
Pass by value to process_input
ianthomas23 Oct 10, 2025
afd276a
Separate namespace for ANSI code sequences to avoid magic strings
ianthomas23 Oct 10, 2025
b77eb87
alternative_buffer scope object
ianthomas23 Oct 10, 2025
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
Remove m_grabbed
  • Loading branch information
@ianthomas23
ianthomas23 committed Oct 10, 2025
commit f7e4643b378d52b227321553dff01687c340462c
20 changes: 7 additions & 13 deletions src/utils/terminal_pager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "terminal_pager.hpp"

terminal_pager::terminal_pager()
: m_grabbed(false), m_rows(0), m_columns(0), m_start_row_index(0)
: m_rows(0), m_columns(0), m_start_row_index(0)
{
maybe_grab_cout();
}
Expand Down Expand Up @@ -46,11 +46,14 @@ std::string terminal_pager::get_input() const
void terminal_pager::maybe_grab_cout()
{
// Unfortunately need to access _internal namespace of termcolor to check if a tty.
if (!m_grabbed && termcolor::_internal::is_atty(std::cout))
if (termcolor::_internal::is_atty(std::cout))
{
// Should we do anything with cerr?
Copy link
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
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
Member

@JohanMabille JohanMabille Oct 17, 2025 •
edited
Loading

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?

m_cout_rdbuf = std::cout.rdbuf(&m_stringbuf);
m_grabbed = true;
}
else
{
m_cout_rdbuf = std::cout.rdbuf();
}
}

Expand Down Expand Up @@ -98,11 +101,7 @@ bool terminal_pager::process_input(const std::string& input)

void terminal_pager::release_cout()
{
if (m_grabbed)
{
std::cout.rdbuf(m_cout_rdbuf);
m_grabbed = false;
}
std::cout.rdbuf(m_cout_rdbuf);
}

void terminal_pager::render_terminal() const
Expand Down Expand Up @@ -166,11 +165,6 @@ void terminal_pager::scroll(bool up, bool page)

void terminal_pager::show()
{
if (!m_grabbed)
{
return;
}

release_cout();

split_input_at_newlines(m_stringbuf.view());
Expand Down
1 change: 0 additions & 1 deletion src/utils/terminal_pager.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class terminal_pager
void update_terminal_size();


bool m_grabbed;
std::stringbuf m_stringbuf;
std::streambuf* m_cout_rdbuf;
std::vector<std::string> m_lines;
Expand Down