Implement terminal_pager for log subcommand #46

ianthomas23 · 2025-09-19T13:33:17Z

The start of a terminal pager for various git2cpp commands, initially enabled for log subcommand. Fixes #45.

pager.mp4

Supports the following keys to navigate:

d, space                   scroll down a page
u                          scroll up a page
q                          quit pager
down arrow, enter, return  scroll down a line
up arrow                   scroll up a line

If cout is not to a tty or the output is short enough to fit within a single page the pager is not used.

Still to do, probably in separate PRs:

Gracefully deal with lines that are longer than the terminal width.
Searching by regex.

To add to other subcommands, see how it is used in log_subcommand.cpp. The terminal_pager constructor does the required initialisation, and it displays on the show call. I suppose I could have done the show automatically in the destructor but that didn't seem a good idea.

Awaiting a new release of JupyterLite before we can easily try it in the terminal.

JohanMabille · 2025-09-24T04:13:27Z

src/utils/terminal_pager.hpp

+
+
+    bool m_grabbed;
+    std::ostringstream m_oss;


It seems that m_oss is used only to get its internal buffer. Why not directly store a std::stringbuf instead?

Done in 44c04be

JohanMabille · 2025-09-24T07:07:43Z

src/utils/terminal_pager.cpp

+#include "terminal_pager.hpp"
+
+terminal_pager::terminal_pager()
+    : m_grabbed(false), m_rows(0), m_columns(0), m_start_row_index(0)


std::cout.rdbuf(std::cout.rdbuf()) is a noop, so I think we could always capture std::cout buffer, and always restore it in release_cout, even if it was not redirected. This way we could drop m_grabbed and the condition in the implementaion of release_cout.

Yes, done in f7e4643

JohanMabille · 2025-09-24T07:10:29Z

src/utils/terminal_pager.cpp

+    // Unfortunately need to access _internal namespace of termcolor to check if a tty.
+    if (!m_grabbed && termcolor::_internal::is_atty(std::cout))
+    {
+        // Should we do anything with cerr?


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

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:

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.

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.

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 · 2025-09-24T07:14:32Z

src/utils/terminal_pager.hpp

+    void maybe_grab_cout();
+
+    // Return true if should stop pager.
+    bool process_input(const std::string& input);


I think it's fine to take the argument by value; calling it with an rvalue will trigger the string's move constructor.

Done in 0edd017

JohanMabille · 2025-09-24T07:17:36Z

src/utils/terminal_pager.cpp

+    auto end_row_index = m_start_row_index + m_rows - 1;
+
+    std::cout << "\e[2J";  // Erase screen.
+    std::cout << "\e[H";  // Cursor to top.


Should we capture these "magic strings" into constants with expressive names?

Yes, I've added a new namespace in ansi_code.hpp/cpp in afd276a.

JohanMabille · 2025-09-24T07:17:52Z

src/utils/terminal_pager.cpp

+            return false;
+        case '\e':  // ANSI escape sequence.
+            // Cannot switch on a std::string.
+            if (input == "\e[A" || input == "\e[1A]")  // Up arrow.


Same question here.

JohanMabille · 2025-09-24T07:22:00Z

src/utils/terminal_pager.cpp

+    auto new_termios = old_termios;
+    // Disable canonical mode (buffered I/O) and echo from stdin to stdout.
+    new_termios.c_lflag &= (~ICANON & ~ECHO);
+    tcsetattr(fileno(stdin), TCSANOW, &new_termios);


I think this and the call to tcsetattr to restore the initial state line 211 should probably be in a scope object to guarantee the initial state is restored even if an excpetion is thrown.

I've added an alternative_buffer and put it in the same place as the existing cursor_hider. I've used a .cpp file too so that at least one of the OS-specific headers doesn't need to be included in header files.

ianthomas23 added 2 commits September 19, 2025 14:19

Implement terminal_pager for log subcommand

97cf63b

Include cstdint

e083e48

ianthomas23 marked this pull request as ready for review September 22, 2025 07:55

JohanMabille reviewed Sep 24, 2025

View reviewed changes

ianthomas23 added 5 commits October 10, 2025 13:09

Use stringbuf instead of ostringstream

44c04be

Remove m_grabbed

f7e4643

Pass by value to process_input

0edd017

Separate namespace for ANSI code sequences to avoid magic strings

afd276a

alternative_buffer scope object

b77eb87

Implement terminal_pager for log subcommand #46

Are you sure you want to change the base?

Implement terminal_pager for log subcommand #46

Uh oh!

Conversation

ianthomas23 commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohanMabille Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohanMabille Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JohanMabille Sep 24, 2025 •

edited

Loading

JohanMabille Sep 24, 2025 •

edited

Loading