refactor: simplify code by removing unused methods and adjusting visi… by broccolingual · Pull Request #15 · broccolingual/rustui

broccolingual · 2026-03-11T06:22:16Z

…bility

Copilot

Pull request overview

This PR refactors the library by removing unused ANSI helpers/APIs and tightening module/type visibility so that terminal and rendering internals are no longer part of the public surface.

Changes:

Make term/render modules and related types/functions crate-private; remove public re-exports from lib.rs.
Remove the csi! macro and simplify terminal command emission to direct ANSI byte writes.
Refactor framebuffer storage (Vec<Cell> → Box<[Cell]>) and adjust output chunking.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/window.rs	Makes `WindowSizeListener` and its methods private to the module.
src/term.rs	Removes unused cursor commands and switches `Terminal::exec` to locked stdout byte writes; restricts visibility to `pub(crate)`.
src/render.rs	Restricts `RenderThread` visibility and changes lock contention behavior in the render loop.
src/lib.rs	Makes `render`/`term` crate-private and removes their public exports.
src/framebuffer.rs	Changes framebuffer backing storage to a boxed slice and reduces render output chunk size.
src/color.rs	Removes `Color::to_ansi()` convenience API and related tests.
src/attr.rs	Removes `Attr::to_ansi()` convenience API and related tests.
src/macros.rs	Deletes the exported `csi!` macro and its test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T06:26:25Z

src/lib.rs

+#[cfg(target_os = "linux")]
+pub(crate) mod render;
+/// A module for handling terminal colors and attributes.
+#[cfg(target_os = "linux")]
+pub(crate) mod term;


render and term were changed from public modules to pub(crate). This is a breaking change for downstream crates that import these modules/types (including the previously #[macro_export]ed csi! macro that lived under mod macros). If the intent is to make these internals, consider keeping a public compatibility layer (re-exports or deprecated items) or bumping the crate version appropriately and documenting the migration in the release notes/README.

Copilot · 2026-03-11T06:26:25Z

src/lib.rs

+pub(crate) use render::*;
+pub(crate) use term::*;


The change from pub use render::* / pub use term::* to pub(crate) use ... removes Cmd, Terminal, and render-related symbols from the public API surface. If external users are expected to control cursor/screen behavior or rendering directly, this will be a breaking API change; consider keeping the public re-exports (possibly with #[deprecated]) or introducing public wrapper APIs before making the internals crate-private.

Suggested change

pub(crate) use render::*;

pub(crate) use term::*;

pub use render::*;

pub use term::*;

Copilot · 2026-03-11T06:26:26Z

src/attr.rs

@@ -36,15 +36,6 @@ impl Default for Attr {
 }

 impl Attr {


Attr::to_ansi() was removed, which is a breaking change for any downstream code that used it (even though write_ansi remains). Consider keeping to_ansi as a thin wrapper around write_ansi and deprecating it, or ensure the crate versioning/docs clearly communicate the API removal.

Suggested change

impl Attr {

impl Attr {

/// Returns the ANSI escape sequence for this attribute as a `String`.

///

/// This method allocates a new `String`. Prefer [`write_ansi`] when you

/// want to write into an existing buffer to avoid allocations.

#[deprecated(

note = "use `Attr::write_ansi` instead to write into an existing buffer without allocating"

)]

pub fn to_ansi(&self) -> String {

let mut buf = String::new();

self.write_ansi(&mut buf);

buf

}

Copilot · 2026-03-11T06:26:26Z

src/color.rs

 impl Color {
-    /// Convert the color to an ANSI escape code.
-    ///
-    /// Returns an ANSI escape code string for the color.
-    pub fn to_ansi(&self, fg: bool) -> String {
-        let mut buf = String::with_capacity(20);
-        self.write_ansi(fg, &mut buf);
-        buf
-    }
-
    /// Write ANSI escape code directly into an existing buffer, avoiding allocation.
    pub fn write_ansi(&self, fg: bool, buf: &mut String) {
        use std::fmt::Write;


Color::to_ansi() was removed, which is a breaking change for downstream callers. If the goal is to avoid allocations, consider retaining to_ansi as a deprecated convenience wrapper around write_ansi so existing code keeps compiling while encouraging the more efficient API.

Copilot · 2026-03-11T06:26:26Z

src/render.rs

-                        // resetting the frame timer so the next iteration does not
-                        // sleep a full rendering_rate before retrying.
-                        continue;
+                        thread::yield_now(); // Yield to other threads if the back framebuffer is currently locked


In the TryLockError::WouldBlock case, replacing the continue with only yield_now() changes the frame pacing behavior: the loop will proceed to the next iteration and may sleep for the remainder of rendering_rate even though the lock might become available sooner. This can reduce responsiveness under contention. Consider yielding and then retrying immediately (e.g., yield_now() + continue), or otherwise adjusting the frame-timer logic so lock contention doesn't introduce extra sleeps.

Suggested change

thread::yield_now(); // Yield to other threads if the back framebuffer is currently locked

thread::yield_now(); // Yield to other threads if the back framebuffer is currently locked

continue; // Retry immediately instead of proceeding with frame pacing logic

Copilot · 2026-03-11T06:26:27Z

src/framebuffer.rs

 use crate::{Attr, Color};

-const CHUNK_SIZE: usize = 1024;
+const CHUNK_SIZE: usize = 256;


CHUNK_SIZE was reduced from 1024 to 256, but refresh() flushes stdout every time the chunk reaches CHUNK_SIZE. With a smaller chunk this increases the number of write_all + flush calls, which can significantly increase syscall overhead and hurt render performance. Consider keeping a larger chunk size and/or flushing once per frame (after all writes) instead of per chunk.

Suggested change

const CHUNK_SIZE: usize = 256;

const CHUNK_SIZE: usize = 1024;

…ocked

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T07:33:44Z

src/lib.rs

+pub(crate) use render::*;
+pub(crate) use term::*;


render and term are #[cfg(target_os = "linux")], but the pub(crate) use render::*; / pub(crate) use term::*; re-exports are unconditional. This will fail to compile on non-Linux targets because those modules won't exist. Gate these re-exports with the same cfg (or a broader cfg that matches intended platform support).

Copilot · 2026-03-11T07:33:44Z

src/render.rs

                    Err(TryLockError::WouldBlock) => {
-                        // back_fb is locked by draw(); retry immediately without
-                        // resetting the frame timer so the next iteration does not
-                        // sleep a full rendering_rate before retrying.
+                        thread::yield_now(); // Yield to other threads if the back framebuffer is currently locked
                        continue;
                    }


On WouldBlock, the render loop now yield_now() and immediately continues without any sleep/backoff. Because Window::draw() holds the back_fb lock across an arbitrary user-provided closure, this can result in a hot retry loop and elevated CPU usage when drawing takes longer than a frame. Consider adding a small bounded backoff (e.g., short sleep/spin with exponential backoff) or restructuring so the render thread blocks more efficiently when the back buffer is contended.

refactor: simplify code by removing unused methods and adjusting visi…

620d7e0

…bility

Copilot AI review requested due to automatic review settings March 11, 2026 06:22

Copilot started reviewing on behalf of broccolingual March 11, 2026 06:22 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

broccolingual added 2 commits March 11, 2026 07:21

fix: increase CHUNK_SIZE to improve framebuffer performance

2b5c5c7

fix: ensure render thread yields correctly when back framebuffer is l…

3fa893d

…ocked

Copilot AI review requested due to automatic review settings March 11, 2026 07:28

Copilot started reviewing on behalf of broccolingual March 11, 2026 07:28 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

broccolingual merged commit 3544c5b into main Mar 11, 2026
5 checks passed

broccolingual deleted the fix/simplification branch March 11, 2026 07:48

broccolingual had a problem deploying to release March 11, 2026 07:51 — with GitHub Actions Failure

broccolingual deployed to release March 12, 2026 07:06 — with GitHub Actions Active

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: simplify code by removing unused methods and adjusting visi…#15

refactor: simplify code by removing unused methods and adjusting visi…#15
broccolingual merged 3 commits intomainfrom
fix/simplification

broccolingual commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-impl Attr {
+impl Attr {
+    /// Returns the ANSI escape sequence for this attribute as a `String`.
+    ///
+    /// This method allocates a new `String`. Prefer [`write_ansi`] when you
+    /// want to write into an existing buffer to avoid allocations.
+    #[deprecated(
+        note = "use `Attr::write_ansi` instead to write into an existing buffer without allocating"
+    )]
+    pub fn to_ansi(&self) -> String {
+        let mut buf = String::new();
+        self.write_ansi(&mut buf);
+        buf
+    }

	thread::yield_now(); // Yield to other threads if the back framebuffer is currently locked
	thread::yield_now(); // Yield to other threads if the back framebuffer is currently locked
	continue; // Retry immediately instead of proceeding with frame pacing logic

	const CHUNK_SIZE: usize = 256;
	const CHUNK_SIZE: usize = 1024;

Conversation

broccolingual commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants