atiaomar1978-hub commented on PR #25408:
URL: https://github.com/apache/camel/pull/25408#issuecomment-5224312760
## Grok code review
**Verdict: 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.
---
### Architecture
```
SourceViewer (key routing, edit lifecycle)
├── SourceEditHistory (undo/redo snapshots)
├── YamlBlockEditor (block find/move/dup/delete/comment)
├── SourceEditorNavigation (word nav, smart home, word delete)
└── SearchHighlighter (find input + n/N nav in edit mode)
```
Good separation — block logic is testable without spinning up the full TUI.
---
### Strengths
- YAML list-item block ops match JIRA intent (Alt+↑/↓ move, Ctrl+D
duplicate, Ctrl+Shift+K delete, Ctrl+/ comment).
- Undo/redo wired through `recordEditChange()` on all mutating paths
including paste and completion insert.
- Footer hints document primary shortcuts for discoverability.
- Tests use AssertJ and package-private access — follows project conventions.
- Key bindings now use `isKey(KeyCode.*)` pattern consistent with existing
view-mode Ctrl+scroll code.
---
### Minor follow-ups (optional, non-blocking)
| # | Area | Suggestion |
|---|------|------------|
| 1 | Find UX | Document that `n`/`N` navigate matches while find term is
active (consistent with view mode but may conflict with typing). |
| 2 | Find highlight | Apply `SearchHighlighter.applyHighlights` in
`renderEditMode` for visual parity with read-only view. |
| 3 | Undo | Consider coalescing rapid consecutive inserts or skipping
duplicate snapshots on `beforeChange`. |
| 4 | Footer | Add Ctrl+Shift+K and Ctrl+Y hints. |
| 5 | Properties files | JIRA mentions `.properties` line ops —
`yamlListBlocks=false` path exists but lacks dedicated integration test. |
| 6 | Commented blocks | `# - to:` lines no longer match list-item detection
— expected, worth a one-line doc note. |
---
### Test coverage
| Test class | Count | Focus |
|------------|-------|-------|
| `YamlBlockEditorTest` | 10 | Block find/move/dup/delete/comment, non-YAML
fallback |
| `SourceEditHistoryTest` | 6 | Undo/redo/cursor restore |
| `SourceEditorNavigationTest` | 7 | Word boundaries, smart home, word
delete |
| `SourceViewerEditorOpsTest` | 10 | Integration: undo, comment, dup,
delete, Alt+↓, Ctrl+←/→, find, footer |
| `SourceViewerEditTest` | (existing) | Edit mode baseline — still passes |
---
### Recommendation
Ready for human committer review. Suggest reviewers: `davsclaus`, `gnodet`
(recent `SourceViewer` / TUI activity).
_AI-generated Grok review on behalf of atiaomar1978-hub._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
