From: Stephen Hemminger <stephen@networkplumber.org>
To: Kerem Aksu <kerem.aksu@i2i-systems.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH] cmdline: update clear screen behavior
Date: Wed, 14 Jan 2026 10:00:13 -0800 [thread overview]
Message-ID: <20260114100013.57faee61@phoenix.local> (raw)
In-Reply-To: <20260105113529.65969-1-kerem.aksu@i2i-systems.com>
On Mon, 5 Jan 2026 14:35:29 +0300
Kerem Aksu <kerem.aksu@i2i-systems.com> wrote:
> 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>
> ---
Overall looks correct. AI review had some feedback.
I would add a release note just for clarity
### Summary
This patch modifies the Ctrl+L behavior in the cmdline library to clear the entire screen and redisplay the prompt at the top, matching standard bash/readline behavior. Currently, Ctrl+L only redisplays on the current line.
---
### Commit Message ✅
| Check | Status | Notes |
|-------|--------|-------|
| Subject ≤60 chars | ✅ | 40 characters |
| Correct prefix | ✅ | `cmdline:` is correct for lib/cmdline |
| Imperative mood | ✅ | "update" |
| No trailing period | ✅ | |
| Lowercase | ✅ | |
| Body wrapped ≤75 chars | ✅ | |
| Body doesn't start with "It" | ✅ | |
| Signed-off-by | ✅ | Present |
| Acked-by/Tested-by | ✅ | Both present |
---
### Code Review
**Overall Assessment**: The approach is sound. Refactoring `rdline_redisplay` into a parameterized `rdline_reprint` static function maintains API compatibility while adding the new clear-screen behavior.
#### Errors (must fix)
**1. Extra whitespace after comma** (style violation)
Lines 126-129 in the patch have double spaces:
```c
rdline_puts(rdl, vt100_clear_screen); /* two spaces after comma */
rdline_puts(rdl, vt100_homecursor); /* two spaces after comma */
rdline_puts(rdl, vt100_home); /* two spaces after comma */
```
Should be:
```c
rdline_puts(rdl, vt100_clear_screen);
rdline_puts(rdl, vt100_homecursor);
rdline_puts(rdl, vt100_home);
```
#### Warnings (should consider)
**2. Consider order of escape sequences**
The current implementation sends `vt100_clear_screen` then `vt100_homecursor`. Some terminals may already position cursor at home after clear screen (ESC [2J), but the explicit `ESC [H` is safer for portability. This is fine as-is.
**3. Potential documentation update**
Consider whether the programmer's guide (`doc/guides/prog_guide/cmdline.rst`) or API documentation needs updating to reflect the changed Ctrl+L behavior. Users familiar with the old behavior might notice the change.
#### Info (observations)
**4. Good API preservation**
The `RTE_EXPORT_SYMBOL(rdline_redisplay)` is correctly maintained, and the public function signature is unchanged. This preserves backward compatibility for any external callers.
**5. New VT100 constant placement**
The new `vt100_homecursor` is added in a reasonable location in `cmdline_vt100.h`, near related cursor movement sequences.
**6. Existing style quirks preserved**
The file already uses `for (i=0 ; i<...` style (spaces around semicolons but not around `=` and `<`). The patch follows this existing convention, which is correct.
---
### Verdict
**Needs revision** - Fix the double-space style issue, then this is ready.
The change itself is a sensible UX improvement that aligns DPDK's command line behavior with user expectations from bash and readline. The implementation is clean and maintains API compatibility.
next prev parent reply other threads:[~2026-01-14 18:00 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-05 11:35 [PATCH] cmdline: update clear screen behavior Kerem Aksu
2026-01-05 15:07 ` Marat Khalili
2026-01-05 20:27 ` Kerem Aksu
2026-01-14 18:00 ` Stephen Hemminger [this message]
2026-01-16 15:42 ` [PATCH v3] " Kerem Aksu
2026-02-10 0:10 ` Stephen Hemminger
2026-02-12 0:23 ` Stephen Hemminger
2026-02-12 7:56 ` Kerem Aksu
2026-02-14 19:08 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260114100013.57faee61@phoenix.local \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=kerem.aksu@i2i-systems.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.