Visitar URL original
Implement terminal_pager for log subcommand by ianthomas23 · Pull Request #46 · QuantStack/git2cpp · GitHub
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

Merged
ianthomas23 merged 7 commits into from
Oct 20, 2025
Merged

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
Use stringbuf instead of ostringstream
  • Loading branch information
@ianthomas23
ianthomas23 committed Oct 10, 2025
commit 44c04bea613db627816467e28e0060a082deb53a
4 changes: 2 additions & 2 deletions src/utils/terminal_pager.cpp
View file
Open in desktop
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?

@JohanMabille JohanMabille Sep 24, 2025

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

@ianthomas23 ianthomas23 Oct 10, 2025

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.

@JohanMabille JohanMabille Oct 17, 2025 •
edited
Loading

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.

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?

@ianthomas23 ianthomas23 Oct 20, 2025

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
View file
Open in desktop
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