Merge pull request #440 from r3bl-org/terminal_async_bug_fix

Fix SharedWriter indentation bug and improve readline type safety

Author: nazmulidris

.claude/skills/check-bounds-safety/SKILL.md

@@ -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
@@ -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,
 };
 ```
 
@@ -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
 
@@ -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
@@ -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:
@@ -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`

.claude/skills/organize-modules/SKILL.md

@@ -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

.vscode/extensions.json

@@ -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.

.vscode/settings.json

@@ -42,6 +42,7 @@
     "Cloneable",
     "cmark",
     "codegen",
+    "codepaths",
     "codepoint",
     "codepoints",
     "col_index",
@@ -318,6 +319,7 @@
     "unergonomic",
     "uninteractive",
     "unparseable",
+    "Unrepresentable",
     "unspecial",
     "unstaged",
     "ustr",