Act IV opens: The Selection (visual mode) + Inner Sanctum (inner text objects)#9
Conversation
…\"/i(/ip) The Archives begin: 24 of 29 curriculum lessons shipped. Engine: - Charwise visual mode: v anchors a selection, a motion subset shapes it, d/x/y/c operate on it (multi-row splice; y degrades to linewise across rows), esc/v leaves; undo restores the selection's start - Inner text objects beyond iw: i\" (quoted span, or the next one ahead, like Vim), i( (innermost pair, nesting-aware, line-local), ip (the paragraph block, blanks preserved; cip collapses to one line) - AllowedKeys gate now skips object/motion completions of an operator already in flight — di( no longer costs a heart for the \"(\" UI: - Selection is reverse-video highlighted in the buffer; -- VISUAL -- HUD - Act IV scaffolding: brass palette, THE ARCHIVES name/summary, welcome screen and title tagline now speak of four acts Content rules: - Act range widened to 1..4; a boss may sit only on its act's final lesson, and every act except the highest (under construction) must end with one. The finale test now asserts mid-game bosses do NOT trigger the finale; the full check returns with The Macro Forge Lessons: The Selection (act4-23, v/vy/vc) and Inner Sanctum (act4-24, ci\"/di(/dip), solutions verified at par.
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds a fourth curriculum act ("The Archives") introducing Visual mode and additional inner-text-object operators (quotes, parentheses, paragraphs) to the Vim emulation engine, wires corresponding UI rendering/HUD/theming, adds two new lesson JSON files, and updates docs and tests to reflect 24 shipped lessons and revised boss-placement rules. ChangesAct IV Feature: Visual Mode + Inner Objects
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant Simulator
participant pressVisual
participant deleteSelection
participant yankSelection
User->>Simulator: press("v")
Simulator->>Simulator: set Mode=ModeVisual, VisualAnchor=cursor
User->>Simulator: press motion keys
Simulator->>pressVisual: dispatch keys
User->>Simulator: press("d"/"x")
pressVisual->>deleteSelection: delete selected span
deleteSelection-->>Simulator: updated Buffer/Cursor
User->>Simulator: press("y")
pressVisual->>yankSelection: yank selected span
yankSelection-->>Simulator: updated register/Cursor
sequenceDiagram
participant Simulator
participant applyOperator
participant innerQuote
participant innerParen
participant innerParagraph
participant cutInnerSpan
Simulator->>applyOperator: press("d"/"c" + "i" + target)
applyOperator->>innerQuote: target == "\""
applyOperator->>innerParen: target == "(" / ")" / "b"
applyOperator->>innerParagraph: target == "p"
innerQuote->>cutInnerSpan: cut(a, b)
innerParen->>cutInnerSpan: cut(a, b)
cutInnerSpan-->>Simulator: updated Buffer/Cursor/Mode
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…, visual gating, rune-safe highlight - innerParen guards the empty line (di( panicked; regression test added) - The allowedKeys bypass is now exactly as wide as an armed inner-object prefix: delimiters complete di"/ci( freely, but an armed operator no longer smuggles count digits past the gate (test added) - Visual-mode commands respect allowedKeys like normal-mode ones (esc free) - renderSelected slices by rune, matching the cursor-render path - Finale test fails loudly if no boss lesson exists - README: palette list gains archive brass; boss claim scoped to Act IV's under-construction state
|
CodeRabbit CLI review disposition (7 findings):
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/LESSONS.md`:
- Line 13: The inline schema example is inconsistent with the updated lesson
naming rule, because it still describes act as 1..3 while the new rule in the
same section says ACT is 1–4. Update the schema example in the lessons guide so
the act range matches the current rule, keeping the reference aligned with the
lesson ordering description that mentions ORDER and sorting by (act, order).
In `@internal/ui/app_test.go`:
- Around line 245-252: The boss-selection setup in the test can leave last as -1
when no lesson has a Boss, which later causes awardBoss() to index m.lessons
with an invalid lessonIdx. Add a guard after the loop in the test around
m.lessonIdx/m.inBoss assignment (using the same pattern as
TestNextActAnnouncementOnBossClear) and fail the test with a clear message if no
boss lesson is found, so the failure happens before any invalid indexing.
In `@README.md`:
- Line 3: The README overview is out of sync with the new four-act tagline
because the journey map still ends at Act III. Update the primary overview
section to include the added Act IV so the headline and the act breakdown match,
using the existing journey map/act section in README as the place to extend.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69692c7e-1aee-42f3-9622-f7a564ae91fb
⛔ Files ignored due to path filters (1)
docs/lessons.csvis excluded by!**/*.csv
📒 Files selected for processing (18)
README.mdassets/lessons/act4-23-the-selection.jsonassets/lessons/act4-24-inner-sanctum.jsondocs/LESSON-GAP-ANALYSIS.mddocs/LESSONS.mdinternal/content/loader_test.gointernal/content/solvable_test.gointernal/engine/edit.gointernal/engine/engine.gointernal/engine/inner_objects_test.gointernal/engine/visual.gointernal/engine/visual_test.gointernal/ui/app_test.gointernal/ui/cmdline_hud_test.gointernal/ui/room.gointernal/ui/styles.gointernal/ui/title.gointernal/ui/welcome.go
| ``` | ||
|
|
||
| `ACT` is 1–3 and `ORDER` is the global lesson order (lessons are sorted by `(act, order)`). | ||
| `ACT` is 1–4 and `ORDER` is the global lesson order (lessons are sorted by `(act, order)`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the schema example to match the new act range.
The file-naming rule now says ACT is 1–4, but the inline schema example still says act is 1..3, so the guide contradicts itself.
Suggested fix
- "act": 2, // 1..3
+ "act": 2, // 1..4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `ACT` is 1–4 and `ORDER` is the global lesson order (lessons are sorted by `(act, order)`). | |
| `ACT` is 1–4 and `ORDER` is the global lesson order (lessons are sorted by `(act, order)`). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/LESSONS.md` at line 13, The inline schema example is inconsistent with
the updated lesson naming rule, because it still describes act as 1..3 while the
new rule in the same section says ACT is 1–4. Update the schema example in the
lessons guide so the act range matches the current rule, keeping the reference
aligned with the lesson ordering description that mentions ORDER and sorting by
(act, order).
Summary
The Archives open — 24 of 29 curriculum lessons shipped. Two medium engine features land: charwise visual mode and the inner text objects every vim daily-driver uses.
New lessons
v+ motions +d/y/con the selectionci"di(dipEngine
vanchors;h j k l w b e 0 $ ^shape the selection;d/x/y/coperate on it;esc/vleave. Multi-row deletes splice; multi-row yanks degrade to linewise (documented); undo restores the selection's start. Selection is reverse-video highlighted and the HUD shows-- VISUAL --i"picks the surrounding (or next) quoted span like Vim;i(finds the innermost pair, nesting-aware;iptakes the paragraph block, preserving surrounding blanks;cvariants insertallowedKeys— previouslydi('s(cost a heart. Same rationale as the existingr/fpending-char bypassv j dleaves both surrounding spaces)Act IV scaffolding
THE ARCHIVES, act summary, four-act welcome + title copyTest plan
go test ./...green (24 lessons, all solvable at par)go vet,gofmt -lcleanversionsmoke runSummary by CodeRabbit