CAMEL-24372: camel-tui YAML editor undo/redo, block ops, word nav, find in edit - #25408
Conversation
|
Reopened on correctly named branch |
Grok code reviewVerdict: Approve with minor follow-ups CAMEL-24372 is implemented cleanly with appropriate helper extraction and solid test coverage. The tamboui KeyEvent pitfalls (modifier + semantic key helpers) were caught and fixed before this PR. ArchitectureGood separation — block logic is testable without spinning up the full TUI. Strengths
Minor follow-ups (optional, non-blocking)
Test coverage
RecommendationReady for human committer review. Suggest reviewers: AI-generated Grok review on behalf of atiaomar1978-hub. |
Bugbot reviewReviewed ScopeYAML/plain-text edit mode enhancements for Verified (looks good)
Issues found & fixed in branch
Remaining suggestions (non-blocking)
Bugbot verdict: No blockers remaining. Approve with minor follow-ups. AI-generated Bugbot review on behalf of atiaomar1978-hub. |
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for this contribution, Omar — nice work on the block-aware editing! The decomposition into SourceEditHistory, YamlBlockEditor, and SourceEditorNavigation is clean and well-tested.
A few items to flag:
Confirmed Issues
-
n/Nfind navigation conflicts with normal typing in edit mode — When a find term is active, pressingnorNnavigates matches instead of inserting the character. This is the view-mode convention but is surprising in edit mode where users expect to type freely. Consider requiringCtrl+N/Ctrl+Shift+Nin edit mode, or only consumingn/Nwhile the find input bar is visible. -
moveBlockDowncursor follows the wrong block —moveBlockUpexplicitly corrects the cursor position (cursorRow = previous.startRow()), butmoveBlockDownreturns the rawswapBlocksresult which lands onsecond.startRow()— that's where the other block landed, not where the user's block went. The cursor should follow the moved block.
Questions
- The
editText()andeditState()package-private accessors onSourceViewer— are these only for test access? If so that's fine, just confirming. viewer.setValidateOnSave(false)inSourceViewerEditorOpsTest— does that method exist on main, or is it also added by this PR? I don't see it in the diff.
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of davsclaus
|
Thanks @davsclaus for the review — addressed both confirmed issues in Confirmed issues
Questions
Ready for another look when you have time. AI-generated comment on behalf of atiaomar1978-hub. |
|
Is the F1 help doc for this page updated |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 2 tested, 1 compile-only — current: 2 all testedMaveniverse Scalpel detected 3 affected modules (current approach: 2).
|
davsclaus
left a comment
There was a problem hiding this comment.
Nice work, Omar — the decomposition into SourceEditHistory, YamlBlockEditor, and SourceEditorNavigation is clean and well-tested. Both prior review issues (n/N conflict and moveBlockDown cursor) are properly fixed.
Minor nits (non-blocking)
-
Home key logic duplicated — the inline Home handler in
SourceViewerreimplementsSourceEditorNavigation.smartHome()usingYamlBlockEditor.leadingSpaces(). CallingsmartHome(editState, false)would remove the duplication. -
positionCursorhoused inSourceEditHistory— it's used fromSourceEditorNavigation,YamlBlockEditor, andSourceVieweras a general cursor utility unrelated to undo/redo. Could move toSourceEditorNavigationor a shared utility in a future tidy-up.
Positive highlights
- Progressive Esc flow (close find input → clear find term → discard confirmation) is good UX
- Find term persisting from view mode into edit mode (
closeInputOnly()replacingsearch.reset()) is a nice touch Ctrl+N/Ctrl+Shift+Nfor find navigation avoids the n/N typing conflict cleanly- 4 test classes with solid coverage
This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of davsclaus
…edit Add SourceEditHistory, YamlBlockEditor, and SourceEditorNavigation helpers and wire them into SourceViewer edit mode for undo/redo (Ctrl+Z/Y), YAML block move/duplicate/delete (Alt+arrows, Ctrl+D/K), comment toggle (Ctrl+/), word navigation/delete, smart Home, and find while editing. Includes comprehensive unit and integration tests for the new editor ops. Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
Use isKey(KeyCode.*) with modifiers for Alt/Ctrl chords (tamboui only matches unmodified keys on isUp/isLeft/isDelete*). Align find n/N with view mode, jump to nearest match on confirm, clear find on Esc, route paste to find input, and fix moveBlockUp cursor placement. Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
Use Ctrl+N / Ctrl+Shift+N for find match navigation in edit mode so plain n/N can be typed while a find term is active. Fix moveBlockDown cursor to follow the moved block (same intent as moveBlockUp). Document test-only editText/editState accessors and add footer hints for redo/delete/find nav. Co-authored-by: Omar Atie <atiaomar1978-hub@users.noreply.github.com>
8559ab1 to
ca667a3
Compare
…y, add F1 help - Move positionCursor from SourceEditHistory to SourceEditorNavigation where it belongs as a general cursor utility - Replace inline Home key logic in SourceViewer with smartHome() call - Add edit mode shortcuts section to F1 help text Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
F1 help text updated in Claude Code on behalf of davsclaus |
|
Addressed review nits in
Claude Code on behalf of davsclaus |
Summary
Implements CAMEL-24372: enhanced plain-text edit mode in the camel-jbang TUI source viewer for YAML (and other local files).
Editor features
#comment on blockImplementation
SourceEditHistory— snapshot-based undo/redo stack (max 100)YamlBlockEditor— YAML list-item block detection and move/duplicate/delete/commentSourceEditorNavigation— word boundaries and smart homeSourceViewer— key bindings, footer hints, find-while-editing viaSearchHighlighterSearchHighlighter— edit-mode find input/navigation,closeInputOnly(),jumpToNearestMatchon confirmReview follow-ups addressed
isKey(KeyCode.*)instead ofisUp()/isLeft()/isDelete*()with Ctrl/Alt)n/Nnavigation aligned with view mode; Esc clears find; paste routes to find input; jump to nearest match after confirmmoveBlockUpcursor lands on the moved block after swapTests
YamlBlockEditorTest— block find/move/duplicate/delete/commentSourceEditHistoryTest— undo/redo stack behaviorSourceEditorNavigationTest— word nav and smart homeSourceViewerEditorOpsTest— integration viaSourceVieweredit modeAI-generated PR description on behalf of atiaomar1978-hub.