Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
119 changes: 119 additions & 0 deletions .claude/skills/check-bounds-safety/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ Follow these principles when working with indices and lengths:
- Instead of `== 0`
- More idiomatic with newtype wrappers

5. **Distinguish navigation from measurement**
- **Navigation** (`index - offset → index`): Moving backward in position space
- **Measurement** (`index.distance_from(other) → length`): Calculating distance between positions
- Use `-` for cursor movement, use `distance_from()` for calculating spans

## Common Imports

```rust
Expand All @@ -82,6 +87,9 @@ use r3bl_tui::{

// Type constructors
col, row, width, height, idx, len,

// Terminal delta types (relative cursor movement)
TermRowDelta, TermColDelta, term_row_delta, term_col_delta,
};
```

Expand All @@ -95,6 +103,7 @@ use r3bl_tui::{
| **Range validation** | `RangeBoundsExt` | `range.check_range_is_valid_for_length(len)` | Iterator bounds, algorithm parameters |
| **Range membership** | `RangeBoundsExt` | `range.check_index_is_within(index)` | VT-100 scroll regions, text selections |
| **Range conversion** | `RangeConvertExt` | `inclusive_range.to_exclusive()` | Converting VT-100 ranges for Rust iteration |
| **Relative movement** | `TermRowDelta`/`TermColDelta` | `TermRowDelta::new(n)` returns `Option` | ANSI cursor movement preventing CSI zero bug |

## Detailed Examples

Expand Down Expand Up @@ -236,6 +245,80 @@ for line in rust_range {
}
```

### Example 7: Navigation vs Measurement

**Use `-` for navigation (moving cursor), `distance_from()` for measurement (calculating spans).**

```rust
use r3bl_tui::{row, height, RowIndex, RowHeight};

// Navigation: Move cursor backward by offset (returns RowIndex).
let cursor_pos = row(10);
let new_pos = cursor_pos - row(3); // row(7) - moved 3 positions back
// Uses saturating subtraction: row(2) - row(5) = row(0), not overflow

// Measurement: Calculate distance between two positions (returns RowHeight).
let start = row(5);
let end = row(15);
let distance: RowHeight = end.distance_from(start); // height(10) - 10 rows apart
// Panics if start > end (negative distance)
```

**When to use which:**
- Moving cursor up/down/left/right → `-` operator
- Calculating scroll amount, viewport span, selection size → `distance_from()`

### Example 8: Terminal Cursor Movement (Make Illegal States Unrepresentable)

**Use `TermRowDelta`/`TermColDelta` for relative cursor movement in ANSI sequences.**

The CSI zero problem: ANSI cursor movement commands interpret parameter 0 as 1:
- `CSI 0 A` (`CursorUp` with n=0) moves cursor **1 row up**, not 0
- `CSI 0 C` (`CursorForward` with n=0) moves cursor **1 column right**, not 0

**Solution:** `TermRowDelta` and `TermColDelta` wrap `NonZeroU16` internally, making zero-valued
deltas **impossible to represent**. Construction is fallible:

```rust
use r3bl_tui::{TermRowDelta, TermColDelta, CsiSequence};
use std::io::Write;

// Calculate cursor movement from position on 80-column terminal.
let position: u16 = 240; // 240 chars from start
let term_width: u16 = 80;

// Fallible construction - must handle the None case.
// For position 240: rows = 3 (Some), cols = 0 (None).
if let Some(delta) = TermRowDelta::new(position / term_width) {
// delta is guaranteed non-zero, safe to emit
term.write_all(CsiSequence::CursorDown(delta).to_string().as_bytes())?;
}
if let Some(delta) = TermColDelta::new(position % term_width) {
// This branch is NOT taken for position 240 (cols = 0)
// Zero cannot be represented, so the bug is prevented at the type level!
term.write_all(CsiSequence::CursorForward(delta).to_string().as_bytes())?;
}
```

**Using the ONE constant for common case:**

```rust
use r3bl_tui::{TermRowDelta, TermColDelta, CsiSequence};

// For the common case of moving exactly 1 row/column, use the ONE constant.
// This avoids the need for fallible construction or unwrap().
let up_one = CsiSequence::CursorUp(TermRowDelta::ONE);
let right_one = CsiSequence::CursorForward(TermColDelta::ONE);
```

**Mathematical law:**
- Zero deltas **cannot exist** - prevented at compile time by `NonZeroU16` wrapper
- `new()` returns `None` for zero, `Some(delta)` for non-zero

**Key difference from absolute positioning:**
- `TermRow`/`TermCol`: 1-based absolute coordinates (for `CursorPosition`)
- `TermRowDelta`/`TermColDelta`: Relative movement amounts (for `CursorUp/Down/Forward/Backward`)

## Decision Trees

See the accompanying `decision-trees.md` file for flowcharts showing which trait to use for
Expand Down Expand Up @@ -309,6 +392,41 @@ if row < width { // Compile error! Can't compare RowIndex with ColWidth

This is actually GOOD - the type system prevents nonsensical comparisons!

### ❌ Mistake 4: Using `-` when you need distance

```rust
// Bad - using subtraction to calculate distance.
// This returns RowIndex, not RowHeight!
let current_row = row(5);
let target_row = row(15);
let rows_to_scroll = target_row - current_row; // Returns row(10), a position!
```

**Fix:**
```rust
// Good - use distance_from() for measurement.
let rows_to_scroll: RowHeight = target_row.distance_from(current_row); // height(10)
```

### ❌ Mistake 5: Emitting CSI zero for cursor movement

```rust
// Bad - CSI 0 C moves 1 column right, not 0!
// This won't even compile now - CursorForward requires TermColDelta, not u16!
let cols = position % term_width; // Could be 0!
let seq = CsiSequence::CursorForward(cols); // Compile error!
```

**Fix:**
```rust
// Good - use fallible delta construction (zero is unrepresentable)
if let Some(delta) = TermColDelta::new(position % term_width) {
// delta is guaranteed non-zero
term.write_all(CsiSequence::CursorForward(delta).to_string().as_bytes())?;
}
// No sequence emitted when cols = 0 (correct behavior - branch not taken)
```

## Reporting Results

When applying bounds checking:
Expand Down Expand Up @@ -343,3 +461,4 @@ No dedicated command, but used throughout the codebase for safe index/length han
- Main implementation: `tui/src/core/units/bounds_check/mod.rs`
- Type definitions: `tui/src/core/units/`
- Examples in tests: `tui/src/core/units/bounds_check/tests/`
- Terminal delta types: `tui/src/core/coordinates/vt_100_ansi_coords/term_row_delta.rs` and `term_col_delta.rs`
88 changes: 88 additions & 0 deletions .claude/skills/organize-modules/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,94 @@ pub mod serialization;

**Why:** Users need to know which features enable which APIs.

### Inner Modules vs. Separate Files

When organizing code into logical groups, choose between **inner modules** (same file) and
**separate files** based on file size and complexity.

#### Inner Modules (Same File)

**✅ Use inner modules when:**

- File is small-to-medium (under ~300 lines total)
- Groups are logically related and benefit from proximity
- Comment banners (`// ======`) are being used to separate sections
- Each group is relatively small (~20-50 lines)

```rust
// ansi_sequence_generator.rs - Inner module pattern

pub struct AnsiSequenceGenerator;

mod cursor_movement {
use super::*;
impl AnsiSequenceGenerator {
pub fn cursor_position(...) -> String { ... }
pub fn cursor_to_column(...) -> String { ... }
}
}

mod screen_clearing {
use super::*;
impl AnsiSequenceGenerator {
pub fn clear_screen() -> String { ... }
pub fn clear_current_line() -> String { ... }
}
}

mod color_ops {
use super::*;
impl AnsiSequenceGenerator {
pub fn fg_color(...) -> String { ... }
pub fn bg_color(...) -> String { ... }
}
}
```

**Benefits:**
- Single-file cohesion - everything related stays together
- Easier navigation - no jumping between files
- Clear grouping - `mod` keyword is more formal than comment banners
- Scoped imports - each inner mod can import only what it needs

#### Separate Files

**✅ Use separate files when:**

- Individual groups exceed ~100 lines each
- Groups have distinct dependencies (different imports)
- File would exceed ~500 lines total
- Groups are conceptually independent (could be tested separately)

```
generator/
├── mod.rs # Re-exports + struct definition
├── cursor_movement.rs # impl AnsiSequenceGenerator { cursor_* }
├── screen_clearing.rs # impl AnsiSequenceGenerator { clear_* }
├── color_ops.rs # impl AnsiSequenceGenerator { colors }
└── terminal_modes.rs # impl AnsiSequenceGenerator { modes }
```

#### Code Smell: Comment Banners

If you find yourself writing comment banners like this:

```rust
impl MyStruct {
// ==================== Group A ====================
fn method_a1() { ... }
fn method_a2() { ... }

// ==================== Group B ====================
fn method_b1() { ... }
fn method_b2() { ... }
}
```

**This is a signal to formalize the grouping** using either inner modules (small file) or
separate files (large file). Comment banners are informal and don't provide the same benefits
as actual module boundaries (scoped imports, clear boundaries, IDE navigation).

## Complete Examples

### Example 1: Simple Module Organization
Expand Down
10 changes: 10 additions & 0 deletions .vscode/extensions.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@
//
// List of extensions which should be recommended for users of this workspace.
"recommendations": [
// R3BL Extensions.
"R3BL.r3bl-auto-insert-copyright",
"R3BL.r3bl-copy-selection-path-and-range",
"R3BL.r3bl-extension-pack",
"R3BL.r3bl-fuzzy-search",
"R3BL.r3bl-semantic-config",
"R3BL.r3bl-shared",
"R3BL.r3bl-task-management",
"R3BL.r3bl-theme",
// Others.
"ryuta46.multi-command", // Multi Command.
"streetsidesoftware.code-spell-checker", // [SLOW] Code Spell Checker. Configure it so it doesn't slow down IDE
"usernamehw.errorlens", // Error Lens.
Expand Down
2 changes: 2 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"Cloneable",
"cmark",
"codegen",
"codepaths",
"codepoint",
"codepoints",
"col_index",
Expand Down Expand Up @@ -318,6 +319,7 @@
"unergonomic",
"uninteractive",
"unparseable",
"Unrepresentable",
"unspecial",
"unstaged",
"ustr",
Expand Down
Loading