-
Notifications
You must be signed in to change notification settings - Fork 3
[PWCI] "cmdline: update clear screen behavior" #588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Control+L should clear screen and redisplay prompt at the top line. DPDK rdline library will not change anything on the screen when fed with Control+L. When prompt is lost after too many text written to the terminal, users will press Control+L to clear screen and put the prompt to the top line. This is expected behavior in bash(1) or applications that are using readline(3). Updated to behave as users expected. Signed-off-by: Kerem Aksu <kerem.aksu@i2i-systems.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's GuideRefactors the rdline redisplay logic to support a dedicated clear‑screen behavior on Ctrl‑L by introducing a reusable reprint helper that can optionally clear the terminal and by adding a new VT100 home-cursor sequence constant. Sequence diagram for updated Ctrl_L clear screen behaviorsequenceDiagram
participant User
participant rdline as rdline_char_in
participant reprint as rdline_reprint
User->>rdline: rdline_char_in(rdl, CMDLINE_KEY_CTRL_L)
activate rdline
rdline->>reprint: rdline_clear_screen(rdl) -> rdline_reprint(rdl, 1)
activate reprint
reprint->>reprint: rdline_puts(rdl, vt100_clear_screen)
reprint->>reprint: rdline_puts(rdl, vt100_homecursor)
reprint->>reprint: write prompt and left buffer
reprint->>reprint: display_right_buffer(rdl, 1)
deactivate reprint
deactivate rdline
Class diagram for updated rdline redisplay helpersclassDiagram
class rdline_module {
+rdline_char_in(rdl, c) int
+rdline_redisplay(rdl) void
-rdline_reprint(rdl, clear_screen) void
-rdline_clear_screen(rdl) void
+display_right_buffer(rdl, force) void
}
class vt100_constants {
+vt100_home string
+vt100_clear_screen string
+vt100_homecursor string
+vt100_word_left string
+vt100_word_right string
}
rdline_module ..> vt100_constants : uses
Flow diagram for rdline_reprint clear screen logicflowchart TD
A[start rdline_reprint] --> B{rdl is NULL?}
B -- yes --> C[return]
B -- no --> D{clear_screen == 1?}
D -- yes --> E[rdline_puts vt100_clear_screen]
E --> F[rdline_puts vt100_homecursor]
D -- no --> G[rdline_puts vt100_home]
F --> H[write prompt characters]
G --> H[write prompt characters]
H --> I[iterate left buffer and write_char]
I --> J["display_right_buffer(rdl, 1)"]
J --> K[end rdline_reprint]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe changes refactor the line editing redisplay logic by introducing internal helper functions that separate screen-clearing behavior from generic redisplay. A new VT100 home-cursor escape sequence macro is added to support this refactoring. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Consider using a bool (e.g.,
bool clear_screen) instead of anintfor theclear_screenparameter inrdline_reprintto better reflect its semantics and improve readability. rdline_clear_screenis a static wrapper used only once; you could callrdline_reprint(rdl, 1)directly in theCMDLINE_KEY_CTRL_Lcase to reduce one layer of indirection unless you expect additional callers in the near future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using a bool (e.g., `bool clear_screen`) instead of an `int` for the `clear_screen` parameter in `rdline_reprint` to better reflect its semantics and improve readability.
- `rdline_clear_screen` is a static wrapper used only once; you could call `rdline_reprint(rdl, 1)` directly in the `CMDLINE_KEY_CTRL_L` case to reduce one layer of indirection unless you expect additional callers in the near future.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
NOTE: This is an auto submission for "cmdline: update clear screen behavior".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36957" for details.
Summary by Sourcery
Update cmdline line-editing behavior so Ctrl-L clears the screen and redraws the current prompt and input line.
Enhancements:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.