[Aintdev]

Summary

Replaces manual enable_raw_mode() / disable_raw_mode() calls with an RAII
RawModeGuard type, so terminal state is restored automatically on scope
exit — including early returns and exceptions — instead of relying on every
call site to clean up manually.

Closes #24

Changes

• Add Source/ctui_raw_mode.h / ctui_raw_mode.cpp with RawModeGuard
  (non-copyable, non-movable). Enables raw mode on construction, restores
  the original terminal state on destruction.
• Fix IXON being cleared on the wrong termios field (c_lflag instead of
  c_iflag) as part of the move.
• Switch to the alternate screen buffer (\033[?1049h / \033[?1049l)
  instead of a plain clear, so the user's original terminal content/scrollback
  is preserved after the program exits.
• Remove raw_mode global bool and free enable_raw_mode() /
  disable_raw_mode() functions from ctui_config.h/.cpp; drop now-unused
  <termios.h> / <stdio.h> includes there.
• Update Sample/App/Source/Main.cpp to use RawModeGuard rmg; instead of
  the manual enable_raw_mode() call.

Notes

• Alt-screen enter/exit is temporarily bundled into the guard's ctor/dtor;
  marked with a TODO to move into the main loop once #
  lands.

[Copilot]

🟡 Changes recommended

Critical issues remain in public-header exposure and console-mode failure handling.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request replaces manual terminal raw-mode management with an RAII RawModeGuard, including automatic restoration and alternate-screen handling.

Changes:

• Adds cross-platform RawModeGuard support.
• Fixes POSIX IXON handling.
• Removes the legacy raw-mode API and updates the sample.

File summaries

File	Summary and final review findings
Source/ctui_raw_mode.h	Declares the RAII guard. Critical (1 vote): Add it to the public umbrella/generated header.
Source/ctui_raw_mode.cpp	Implements terminal state and alternate-screen handling. Critical (3 votes): Handle invalid or redirected console handles before using mode fields. Moderate (1 vote): Move process-global sync_with_stdio(false) outside the guard. Nit (1 vote): Correct the issue references.
Source/ctui_config.h	Removes legacy raw-mode declarations.
Source/ctui_config.cpp	Removes the legacy implementation and unused includes.
Sample/App/Source/Main.cpp	Uses RawModeGuard. Critical (2 votes): Expose the guard through the documented public header. Nit (2 votes): Update the README’s removed API example. Nit (1 vote): Correct the unrelated #24 closing reference.

Review details

Suppressed comments (5)

Sample/App/Source/Main.cpp:18

• The PR description says Closes #24, but issue #24 is the unrelated Widget::get_parent() null-root bug and is already closed by PR #49; this change only adds raw-mode handling. Please remove or correct the closing reference, or include the intended widget fix.

    RawModeGuard rmg;

Source/ctui_raw_mode.cpp:44

• On Windows, this restores _stdout before the \033[?1049l written on line 48. If the original console mode did not include ENABLE_VIRTUAL_TERMINAL_PROCESSING (the usual case), that exit sequence is no longer interpreted, so the process can leave the terminal in the alternate buffer. Emit and flush the exit sequence while VT processing is still enabled, then restore _stdout.

        SetConsoleMode(_stdout, _original_stdout_mode);

Source/ctui_raw_mode.cpp:26

• tcgetattr can fail for redirected or non-TTY stdin. In that case _original remains uninitialized, so line 28 reads indeterminate state and the destructor later passes it to tcsetattr. Check the return value and either make the guard inactive or fail construction before using the snapshot.

        tcgetattr(STDIN_FILENO, &_original);

Source/ctui_raw_mode.cpp:36

• sync_with_stdio(false) is process-global and is not a terminal-state change; putting it in this scope guard means a short-lived guard permanently changes C/C++ stream synchronization, and calling it after I/O has implementation-defined effects. Move this one-time configuration to application startup or otherwise keep it outside the guard's terminal-state ownership.

        std::ios::sync_with_stdio(false); 

Source/ctui_raw_mode.cpp:35

• The PR description says this closes issue #24 and that missing SIGINT handling is tracked by #25, but those repository issues are actually Widget::get_parent() and TextStream function-token caching (#24 is already closed by PR #49). Please correct the linked issue references so this change is traceable to the raw-mode work.

        // TODO: Move code to main loop as soon as it exists.

• Files reviewed: 5/5 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Critical compile-blocking issues and a terminal-state handling issue remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (7)

Sample/App/Source/Main.cpp:18

• The PR description says Closes #24, but issue #24 is the unrelated Widget::get_parent() null-parent bug and is already closed by PR #49; this diff only changes terminal mode. Please remove or update that closing reference so the PR metadata does not claim to resolve an unrelated issue.

    RawModeGuard rmg;

Source/ctui.h:12

• The PR description says Closes #24, but issue #24 is the unrelated Widget::get_parent() null-root bug; this diff only adds raw-mode handling and does not touch that code. Remove the closing reference or update it to the intended issue so the PR metadata matches the change.

#include "ctui_raw_mode.h"

Source/ctui_raw_mode.cpp:52

• Because the constructor disables stdio synchronization, this only appends the exit sequence to std::cout's buffer. The destructor then restores the Windows console mode before that buffer is guaranteed to flush, so the escape can be emitted after VT processing is disabled (and an uncaught exception can drop it entirely), leaving the terminal in the alternate screen. Flush the stream before restoring the modes.

        std::cout << "\033[?1049l"; // TODO: move to main loop

Source/ctui_raw_mode.cpp:27

• Both SetConsoleMode calls ignore their failure status. If enabling input or VT output fails, construction still succeeds and the guard enters the alternate screen even though raw/VT mode was not established; if the second call fails after the first succeeds, the partial input-mode change also needs to be rolled back before propagating the error. Check both results and handle that partial-update path.

        SetConsoleMode(_stdin, stdin_mode);

        DWORD stdout_mode = _original_stdout_mode;
        stdout_mode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
        SetConsoleMode(_stdout, stdout_mode);

Source/ctui_raw_mode.cpp:38

• The tcsetattr result is ignored, so construction succeeds and the guard enters the alternate screen even when raw-mode setup failed; input then remains canonical/echoing while the guard claims to have enabled raw mode. Check this call and throw before emitting the screen-control sequences when it fails.

        tcsetattr(STDIN_FILENO, TCSANOW, &raw);

Source/ctui_raw_mode.cpp:43

• Because sync_with_stdio(false) is set immediately above, this output can remain buffered; the guard then claims to have entered the alternate screen before it has actually sent the escape sequence. Flush here so construction establishes the promised terminal state even if the caller reads input before its first output.

        std::cout << "\033[?1049h\033[2J\033[H";

Source/ctui_raw_mode.h:25

• The PR description says this change closes issue #24, but that issue is the unrelated Widget::get_parent() root-null dereference and is already closed by PR #49. Please remove the stale closing reference or point this PR at the intended issue so the repository's issue tracking is not misleading.

		RawModeGuard();

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Lite