Fix/alignment in tables #42

zahorodnyi · 2026-01-08T18:29:29Z

Move display table functions to a separate module.
Rework those four functions to use the comfy-table crate; the tables now look better.

Fixes: #37

KyrylR · 2026-01-08T19:10:38Z

greptile-apps

Greptile Overview

Greptile Summary

This PR refactors table display functions into a dedicated tables.rs module and replaces manual string formatting with the comfy-table crate for improved visual presentation.

Major Changes:

Created new tables.rs module containing four display functions: display_token_table, display_swap_table, display_collateral_table, and display_user_token_table
Migrated functions from interactive.rs (2 functions) and positions.rs (2 functions) to the new module
Replaced manual table formatting with comfy-table library using UTF8_FULL preset for better-aligned, bordered tables
Updated imports in browse.rs, interactive.rs, and positions.rs to reference the new module

Code Quality Notes:

The four display functions follow an identical pattern, suggesting opportunity for further refactoring to reduce duplication
Functions previously marked private in positions.rs are now public in tables.rs, which is appropriate for cross-module usage
The refactoring improves code organization by consolidating all table display logic in one place

Confidence Score: 4/5

This PR is safe to merge with minimal risk
Score reflects a well-executed refactoring with clear improvements to code organization and table formatting. The only concern is code duplication in the new module, which is a style issue rather than a functional problem.
crates/cli-client/src/cli/tables.rs could benefit from refactoring to reduce code duplication across the four display functions

Important Files Changed

File Analysis

Filename	Score	Overview
crates/cli-client/Cargo.toml	5/5	Added comfy-table dependency for improved table formatting
crates/cli-client/src/cli/interactive.rs	5/5	Removed display_token_table and display_swap_table functions, moved to tables module
crates/cli-client/src/cli/positions.rs	5/5	Removed display_collateral_table and display_user_token_table functions, moved to tables module
crates/cli-client/src/cli/tables.rs	4/5	New module with four table display functions using comfy-table; functions contain repetitive code that could be refactored

Sequence Diagram

sequenceDiagram
    participant B as browse.rs
    participant I as interactive.rs
    participant P as positions.rs
    participant T as tables.rs
    participant CT as comfy_table
    
    Note over B,P: Before: Functions scattered across modules
    Note over T: After: Centralized in tables.rs
    
    B->>T: display_token_table(tokens)
    activate T
    T->>CT: Table::new()
    T->>CT: load_preset(UTF8_FULL)
    T->>CT: set_header(...)
    loop for each token
        T->>CT: add_row(...)
    end
    T->>CT: to_string()
    T-->>B: print formatted table
    deactivate T
    
    P->>T: display_collateral_table(displays)
    activate T
    T->>CT: Table::new()
    T->>CT: load_preset(UTF8_FULL)
    T->>CT: set_header(...)
    loop for each display
        T->>CT: add_row(...)
    end
    T->>CT: to_string()
    T-->>P: print formatted table
    deactivate T
    
    I->>T: display_token_table(tokens)
    Note over I,T: Reuses same function from tables module

greptile-apps · 2026-01-08T19:14:34Z

crates/cli-client/src/cli/tables.rs

+pub fn display_token_table(tokens: &[TokenDisplay]) {
+    if tokens.is_empty() {
+        println!("  (No tokens found)");
+        return;
+    }
+
+    let mut table = Table::new();
+
+    table.load_preset(UTF8_FULL);
+
+    table.set_header(vec![
+        Cell::new("#").add_attribute(Attribute::Bold),
+        Cell::new("Collateral/Token").add_attribute(Attribute::Bold),
+        Cell::new("Strike/Token").add_attribute(Attribute::Bold),
+        Cell::new("Expires").add_attribute(Attribute::Bold),
+        Cell::new("Contract").add_attribute(Attribute::Bold),
+    ]);
+
+    for token in tokens {
+        table.add_row(vec![
+            token.index.to_string(),
+            token.collateral.clone(),
+            token.settlement.clone(),
+            token.expires.clone(),
+            token.status.clone(),
+        ]);
+    }
+
+    let table_string = table.to_string();
+    for line in table_string.lines() {
+        println!("  {line}");
+    }
+}


significant code duplication across all four display functions - each follows identical pattern (create table, load preset, set header, add rows, print with indentation)

KyrylR · 2026-01-08T19:27:27Z

@greptile review the PR according to the CONTRIBUTION.md

greptile-apps

Greptile Overview

Greptile Summary

This PR refactors table display functions by moving them to a dedicated tables.rs module and implementing the comfy-table crate for improved formatting. The changes include:

Created new tables.rs module with four display functions: display_token_table, display_swap_table, display_collateral_table, and display_user_token_table
Migrated from basic println! formatting to comfy-table with UTF8_FULL preset and bold headers
Updated imports in interactive.rs, positions.rs, and browse.rs to reference the new module
Added comfy-table = "7.2.1" dependency

Critical Issue:
The commit history violates the repository's commit policy. The PR contains 7 commits including two "fix: CI" commits (4f50d3e and e9ea4e6) that correct errors introduced in earlier commits within the same PR. Specifically, commit 823e006 incorrectly called .to_string() on a String field (display.amount.to_string() instead of display.amount.clone()), which was only fixed two commits later in e9ea4e6. This means intermediate commits don't build/pass tests independently.

Confidence Score: 2/5

This PR has a critical commit history issue that violates repository policy, though the final code changes are safe
The code refactoring itself is clean and improves table formatting, but the commit history contains "fix: CI" commits that correct errors introduced in previous commits within the same PR. This violates the repository's commit policy requiring each commit to build and pass tests independently. The commits should be squashed or rebased to create a clean, logical history where each commit is self-contained and functional.
No files require special attention - the code changes are good, but the commit history needs to be cleaned up before merging

Important Files Changed

File Analysis

Filename	Score	Overview
crates/cli-client/Cargo.toml	5/5	added `comfy-table` dependency for improved table formatting
crates/cli-client/src/cli/tables.rs	5/5	new module with four table display functions using `comfy-table` for better formatting
crates/cli-client/src/cli/interactive.rs	5/5	removed `display_token_table` and `display_swap_table` functions (moved to tables.rs)
crates/cli-client/src/cli/positions.rs	5/5	removed `display_collateral_table` and `display_user_token_table` functions (moved to tables.rs)

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant Module as tables.rs (new)
    participant Interactive as interactive.rs
    participant Positions as positions.rs
    participant Browse as browse.rs
    participant ComfyTable as comfy-table crate

    Note over Dev,ComfyTable: Refactoring: Move & Upgrade Table Display Functions

    Dev->>Module: Create new tables.rs module
    Dev->>Interactive: Remove display_token_table()
    Dev->>Interactive: Remove display_swap_table()
    Dev->>Positions: Remove display_collateral_table()
    Dev->>Positions: Remove display_user_token_table()
    
    Dev->>Module: Add display_token_table() with comfy-table
    Dev->>Module: Add display_swap_table() with comfy-table
    Dev->>Module: Add display_collateral_table() with comfy-table
    Dev->>Module: Add display_user_token_table() with comfy-table
    
    Dev->>Interactive: Import display_token_table from tables
    Dev->>Positions: Import all table functions from tables
    Dev->>Browse: Import table functions from tables
    
    Note over Browse,ComfyTable: Runtime: Display Tables
    
    Browse->>Module: Call display_token_table(tokens)
    Module->>ComfyTable: Create Table with UTF8_FULL preset
    ComfyTable-->>Module: Formatted table string
    Module->>Module: Print with indentation
    
    Browse->>Module: Call display_swap_table(swaps)
    Module->>ComfyTable: Create Table with UTF8_FULL preset
    ComfyTable-->>Module: Formatted table string
    Module->>Module: Print with indentation

greptile-apps · 2026-01-08T19:31:03Z

crates/cli-client/src/cli/tables.rs

+use crate::cli::interactive::{SwapDisplay, TokenDisplay};
+use crate::cli::positions::{CollateralDisplay, UserTokenDisplay};
+use comfy_table::presets::UTF8_FULL;
+use comfy_table::{Attribute, Cell, Table};
+
+pub fn display_token_table(tokens: &[TokenDisplay]) {
+    if tokens.is_empty() {
+        println!("  (No tokens found)");
+        return;
+    }
+
+    let mut table = Table::new();
+
+    table.load_preset(UTF8_FULL);
+
+    table.set_header(vec![
+        Cell::new("#").add_attribute(Attribute::Bold),
+        Cell::new("Collateral/Token").add_attribute(Attribute::Bold),
+        Cell::new("Strike/Token").add_attribute(Attribute::Bold),
+        Cell::new("Expires").add_attribute(Attribute::Bold),
+        Cell::new("Contract").add_attribute(Attribute::Bold),
+    ]);
+
+    for token in tokens {
+        table.add_row(vec![
+            token.index.to_string(),
+            token.collateral.clone(),
+            token.settlement.clone(),
+            token.expires.clone(),
+            token.status.clone(),
+        ]);
+    }
+
+    let table_string = table.to_string();
+    for line in table_string.lines() {
+        println!("  {line}");
+    }
+}
+
+pub fn display_swap_table(swaps: &[SwapDisplay]) {
+    if swaps.is_empty() {
+        println!("  (No swaps found)");
+        return;
+    }
+
+    let mut table = Table::new();
+
+    table.load_preset(UTF8_FULL);
+
+    table.set_header(vec![
+        Cell::new("#").add_attribute(Attribute::Bold),
+        Cell::new("Price").add_attribute(Attribute::Bold),
+        Cell::new("Wants").add_attribute(Attribute::Bold),
+        Cell::new("Expires").add_attribute(Attribute::Bold),
+        Cell::new("Seller").add_attribute(Attribute::Bold),
+    ]);
+
+    for swap in swaps {
+        table.add_row(vec![
+            swap.index.to_string(),
+            swap.offering.clone(),
+            swap.wants.clone(),
+            swap.expires.clone(),
+            swap.seller.clone(),
+        ]);
+    }
+
+    let table_string = table.to_string();
+    for line in table_string.lines() {
+        println!("  {line}");
+    }
+}
+
+pub fn display_collateral_table(displays: &[CollateralDisplay]) {
+    if displays.is_empty() {
+        println!("  (No locked assets found)");
+        return;
+    }
+
+    let mut table = Table::new();
+
+    table.load_preset(UTF8_FULL);
+
+    table.set_header(vec![
+        Cell::new("#").add_attribute(Attribute::Bold),
+        Cell::new("Locked Assets").add_attribute(Attribute::Bold),
+        Cell::new("Settlement").add_attribute(Attribute::Bold),
+        Cell::new("Expires").add_attribute(Attribute::Bold),
+        Cell::new("Contract").add_attribute(Attribute::Bold),
+    ]);
+
+    for display in displays {
+        table.add_row(vec![
+            display.index.to_string(),
+            display.collateral.clone(),
+            display.settlement.clone(),
+            display.expires.clone(),
+            display.contract.clone(),
+        ]);
+    }
+
+    let table_string = table.to_string();
+    for line in table_string.lines() {
+        println!("  {line}");
+    }
+}
+
+pub fn display_user_token_table(displays: &[UserTokenDisplay]) {
+    if displays.is_empty() {
+        println!("  (No option/grantor tokens found)");
+        return;
+    }
+
+    let mut table = Table::new();
+
+    table.load_preset(UTF8_FULL);
+
+    table.set_header(vec![
+        Cell::new("#").add_attribute(Attribute::Bold),
+        Cell::new("Type").add_attribute(Attribute::Bold),
+        Cell::new("Amount").add_attribute(Attribute::Bold),
+        Cell::new("Strike/Token").add_attribute(Attribute::Bold),
+        Cell::new("Expires").add_attribute(Attribute::Bold),
+        Cell::new("Contract").add_attribute(Attribute::Bold),
+    ]);
+
+    for display in displays {
+        table.add_row(vec![
+            display.index.to_string(),
+            display.token_type.clone(),
+            display.amount.clone(),
+            display.strike.clone(),
+            display.expires.clone(),
+            display.contract.clone(),
+        ]);
+    }
+
+    let table_string = table.to_string();
+    for line in table_string.lines() {
+        println!("  {line}");
+    }
+}


This PR has commit history issues that violate the repository's commit policy. The commit sequence includes "fix: CI" and "fix: CI 2" commits that fix issues from previous commits in the same PR (commit 823e006 incorrectly used .to_string() on an already-String field, fixed in e9ea4e6). Each commit should build and pass tests independently without needing follow-up fix commits.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

zahorodnyi · 2026-01-09T15:34:27Z

@greptile review the PR according to the CONTRIBUTION.md

greptile-apps · 2026-01-09T15:46:34Z

Greptile Overview

Greptile Summary

This PR refactors table display functionality in the CLI client by extracting four display functions (display_token_table, display_swap_table, display_collateral_table, display_user_token_table) into a dedicated tables.rs module and migrating them from manual string formatting to the comfy-table crate for improved visual presentation.

Key Changes

Module Structure: Creates a new crates/cli-client/src/cli/tables.rs module to centralize table rendering logic, improving code organization and separation of concerns.

Implementation Approach: Introduces a private TableData trait with get_header() and to_row() methods, implemented by four display structs (TokenDisplay, SwapDisplay, CollateralDisplay, UserTokenDisplay). A generic render_table<T: TableData>() function eliminates code duplication across the four display functions.

Visual Improvements: Replaces manual println! formatting with comfy-table's UTF8_FULL preset, providing Unicode box-drawing characters for cleaner, more professional-looking tables. The new implementation automatically handles column width adjustment instead of using fixed-width formatting.

Dependency Addition: Adds comfy-table = "7.2.1" to Cargo.toml.

Commit Quality

The PR follows excellent commit hygiene with 6 logical, incremental commits:

Moves existing functions to new module (preserving old implementation)
Adds comfy-table dependency and migrates first function
3-5. Migrates remaining three functions one at a time
Refactors to eliminate duplication using trait abstraction

Each commit is self-contained and buildable, with no merge commits or reverts.

Architecture Impact

The refactoring maintains all existing public interfaces - callers in browse.rs, interactive.rs, and positions.rs simply update their import paths. The Display structs remain in their original modules with public fields, ensuring no breaking changes. The trait-based design makes future additions of new table types straightforward.

Confidence Score: 5/5

This PR is safe to merge with no identified risks.
After thorough analysis of all 6 files and 6 commits, I found zero bugs, logic errors, or security issues. The refactoring is well-executed with: (1) clean trait-based abstraction eliminating code duplication, (2) proper module organization maintaining all public interfaces, (3) correct implementation of all four table display functions with matching header/row column counts, (4) excellent commit hygiene with logical, incremental, buildable commits per custom instruction requirements, (5) appropriate dependency addition (comfy-table 7.2.1), and (6) consistent behavior preservation including empty message handling and indentation. The code follows Rust best practices, has no breaking changes, and represents a clear improvement in code quality and maintainability.
No files require special attention. All changes are straightforward refactoring with proper execution.

Important Files Changed

File Analysis

Filename	Score	Overview
crates/cli-client/src/cli/tables.rs	5/5	New module implementing table display with comfy-table crate using trait-based generic design. Clean refactoring with proper trait abstraction, correct column counts, and consistent empty message handling.
crates/cli-client/src/cli/interactive.rs	5/5	Removed display_token_table and display_swap_table functions, added import from tables module. Clean extraction with no logic changes to remaining code.
crates/cli-client/src/cli/positions.rs	5/5	Removed display_collateral_table and display_user_token_table functions, added import from tables module. Display structs remain with public fields for trait implementation.
crates/cli-client/src/cli/browse.rs	5/5	Updated imports to reference display functions from tables module instead of interactive. No logic changes, simple import path update.
crates/cli-client/Cargo.toml	5/5	Added comfy-table 7.2.1 dependency for improved table rendering. Version is recent and stable.
crates/cli-client/src/cli/mod.rs	5/5	Added tables module declaration. Straightforward module registration with no issues.

Sequence Diagram

sequenceDiagram
    participant User as User (CLI)
    participant Browse as browse.rs
    participant Positions as positions.rs
    participant Interactive as interactive.rs
    participant Tables as tables.rs (new)
    participant ComfyTable as comfy-table crate
    
    Note over Browse,Tables: Before: display functions in multiple modules
    Note over Tables: After: centralized in tables.rs
    
    User->>Browse: run browse command
    Browse->>Browse: fetch options/swaps
    Browse->>Browse: build TokenDisplay/SwapDisplay
    Browse->>Tables: display_token_table(displays)
    Tables->>Tables: check if empty
    Tables->>Tables: create Table::new()
    Tables->>ComfyTable: load_preset(UTF8_FULL)
    Tables->>ComfyTable: set_header(bold cells)
    loop for each display
        Tables->>ComfyTable: add_row(display.to_row())
    end
    ComfyTable-->>Tables: formatted table string
    Tables->>User: print table with indentation
    
    User->>Positions: run positions command
    Positions->>Positions: build CollateralDisplay/UserTokenDisplay
    Positions->>Tables: display_collateral_table(displays)
    Tables->>ComfyTable: render with UTF8_FULL preset
    ComfyTable-->>Tables: formatted table
    Tables->>User: print table
    
    Positions->>Tables: display_user_token_table(displays)
    Tables->>ComfyTable: render with UTF8_FULL preset
    ComfyTable-->>Tables: formatted table
    Tables->>User: print table
    
    Note over Tables,ComfyTable: Generic render_table<T: TableData>() handles all types

zahorodnyi added 5 commits January 8, 2026 19:03

Move display tables functions to separate module

9937de8

Rework display_token_table to use comfy-table

e68d826

Rework display_swap_table to use comfy-table

ecded29

Rework display_collateral_table to use comfy-table

78e3ec8

Rework display_user_token_table to use comfy-table

823e006

zahorodnyi requested a review from KyrylR as a code owner January 8, 2026 18:29

KyrylR added the greptile Mark this PR to be reviewed by Greptile label Jan 8, 2026

greptile-apps bot reviewed Jan 8, 2026

View reviewed changes

Fix code duplication in table display functions

6d1a5ed

zahorodnyi force-pushed the fix/alignment-in-tables branch from f535797 to 6d1a5ed Compare January 9, 2026 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix/alignment in tables #42

Fix/alignment in tables #42

Uh oh!

zahorodnyi commented Jan 8, 2026 •

edited by KyrylR

Loading

Uh oh!

KyrylR commented Jan 8, 2026

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Jan 8, 2026

Uh oh!

KyrylR commented Jan 8, 2026

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Jan 8, 2026

Uh oh!

zahorodnyi commented Jan 9, 2026

Uh oh!

greptile-apps bot commented Jan 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix/alignment in tables #42

Are you sure you want to change the base?

Fix/alignment in tables #42

Uh oh!

Conversation

zahorodnyi commented Jan 8, 2026 • edited by KyrylR Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KyrylR commented Jan 8, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

KyrylR commented Jan 8, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

zahorodnyi commented Jan 9, 2026

Uh oh!

greptile-apps bot commented Jan 9, 2026

Greptile Overview

Greptile Summary

Key Changes

Commit Quality

Architecture Impact

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zahorodnyi commented Jan 8, 2026 •

edited by KyrylR

Loading