apache
app/system/vi: Fix broken VI editor
Summary
-
Since Nuttx commit https://github.com/apache/nuttx/commit/df5c876932c4c82e8aee32adca651bb99d9d6200 , the serial driver enable ICANON flag as default. This broken the VI and all keystrokes inputs are buffered. So disable it in termcurses.
-
If the VI exits in an error condition it will break the console. This is because release_vi was not called to restore the console configuration, so fix it.
Impact
All VI-enabled boards.
Testing
Test in Ubuntu 24.20 use sim:nsh.
[Experimental Bot, please feedback here]
The PR summary you provided lacks some key details to determine if it meets the NuttX requirements. Here's a breakdown and what needs improvement:
Strengths:
- Clear problem statements: Both points in the summary clearly identify the issue being addressed.
- Impact is addressed: It broadly identifies affected boards.
Weaknesses:
- Missing details about the change itself:
- The summary only describes the problem not the solution. How exactly are you disabling ICANON in termcurses? What code is modified?
- How is
release_vibeing incorporated to fix the error condition exit?
- Impact section is too vague:
- "All VI-enabled boards" is not specific enough. Which architectures? Are there known boards where this won't be an issue?
- The other impact categories are completely skipped. At minimum, address:
- Build: Did the build process change at all due to your modifications?
- Documentation: Does this fix require any documentation updates?
- Testing is insufficient:
- Only one host/target combination: NuttX supports a wide range of platforms. Testing on a single simulated environment is not enough.
- No logs provided: The "Testing logs" sections are empty, making it impossible to verify the fix.
To meet NuttX requirements, the PR needs these improvements:
- Elaborate on the solution: Don't just describe the problem. Explain the code changes made to implement the fix.
- Be specific about impact: List affected architectures and, if possible, any boards known to be unaffected. Address all other impact categories.
- Expand testing:
- Test on at least one more significantly different target (e.g., a real board if you initially tested on simulator).
- Include relevant logs: Show output before and after your change to demonstrate the fix.
By addressing these points, the PR will be much stronger and more likely to be accepted.
@bl4kraven please fix the following style error:
Error: /home/runner/work/nuttx-apps/nuttx-apps/apps/system/termcurses/tcurses_vt100.c:1549:83: error: Long line found
