Reconstruct the shell, manage historical commands using list, and add the ctrl+z/y functions #10986

youzuwei · 2025-12-01T05:44:40Z

拉取/合并请求描述：(PR description)

[

为什么提交这份PR (why to submit this PR)

#10978

你的解决方案是什么 (what is your solution)

重构shell，拆分大函数。使用链表管理历史命令。增加ctrl+z/y功能，可使用宏（FINSH_USING_SNAPSHOT）开启和关闭

请提供验证的bsp和config (provide the config and bsp)

BSP:bsp\stm32\stm32h743-atk-apollo

.config:

action:

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality：

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

已经仔细查看过代码改动的对比 Already check the difference between PR and old code
代码风格正确，包括缩进空格，命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
没有垃圾代码，代码尽量精简，不包含#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up
所有变更均有原因及合理的，并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
代码是高质量的 Code in this PR is of high quality
已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
如果是新增bsp, 已经添加ci检查到.github/ALL_BSP_COMPILE.json 详细请参考链接BSP自查

… the ctrl+z/y functions

github-actions · 2025-12-01T05:44:49Z

👋 感谢您对 RT-Thread 的贡献！Thank you for your contribution to RT-Thread!

为确保代码符合 RT-Thread 的编码规范，请在你的仓库中执行以下步骤运行代码格式化工作流（如果格式化CI运行失败）。
To ensure your code complies with RT-Thread's coding style, please run the code formatting workflow by following the steps below (If the formatting of CI fails to run).

🛠 操作步骤 | Steps

前往 Actions 页面 | Go to the Actions page
点击进入工作流 → | Click to open workflow →
点击 Run workflow | Click Run workflow

设置需排除的文件/目录（目录请以"/"结尾）
Set files/directories to exclude (directories should end with "/")
将目标分支设置为 \ Set the target branch to：master
设置PR number为 \ Set the PR number to：10986

等待工作流完成 | Wait for the workflow to complete
格式化后的代码将自动推送至你的分支。
The formatted code will be automatically pushed to your branch.

完成后，提交将自动更新至 master 分支，关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the master branch automatically, and the related Pull Request will be updated.

如有问题欢迎联系我们，再次感谢您的贡献！💐
If you have any questions, feel free to reach out. Thanks again for your contribution!

github-actions · 2025-12-01T05:44:49Z

📌 Code Review Assignment

🏷️ Tag: components

Reviewers: Maihuanyi

Changed Files (Click to expand)

components/finsh/Kconfig
components/finsh/shell.c
components/finsh/shell.h

📊 Current Review Status (Last Updated: 2025-12-02 13:42 CST)

⌛ Maihuanyi Pending Review

📝 Review Instructions

维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
Maintainers can refresh the review status by clicking here: 🔄 Refresh Status
确认审核通过后评论 LGTM/lgtm
Comment LGTM/lgtm after confirming approval
PR合并前需至少一位维护者确认
PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

…n up comments for consistency.

…e_t to rt_size_t.

wdfk-prog

改太多了,看不过来.
能分批提交么?

wdfk-prog · 2025-12-02T00:37:13Z

components/finsh/shell.c

-struct finsh_shell *shell;
-static char *finsh_prompt_custom = RT_NULL;
+static struct finsh_shell *shell = RT_NULL;
+static char finsh_prompt[RT_CONSOLEBUF_SIZE + 1] = { 0 };


这个咋成静态的了?原来的动态分配不好吗?

只有在使用的时候才分配内存.而不是有没有使用都占用

这里以前也是一个静态数组，放在函数const char *finsh_get_prompt(void) static char finsh_prompt[RT_CONSOLEBUF_SIZE + 1] = {0};中，动态分配存没有使用heap的时候，不能自定义prompt提示词，因为现在使用的自定义提示词是动态内存分配出来的

wdfk-prog · 2025-12-02T00:38:42Z

components/finsh/shell.c

    {
-        finsh_prompt[0] = '\0';
-        return finsh_prompt;
+        finsh_prompt[RT_CONSOLEBUF_SIZE] = 0;


改成\0是不是更好,虽然一样

wdfk-prog · 2025-12-02T00:41:33Z

components/finsh/shell.c

-        finsh_prompt[rt_strlen(finsh_prompt)] = '>';
-        finsh_prompt[rt_strlen(finsh_prompt) + 1] = '\0';
+        finsh_prompt[len++] = '>';
+        finsh_prompt[len++] = 0;


这里没必要再次++了吧

是的，这里可以不用加了，写习惯了

但这里也不影响

wdfk-prog · 2025-12-02T00:42:43Z

components/finsh/shell.c

+    if (!finsh_prompt[0])
+        finsh_set_prompt_word(FINSH_PROMPT_WORD_DEFAULT);

+    len = finsh_prompt_length;


这里依赖与finsh_prompt_length = rt_strlen(finsh_prompt);这个逻辑
-没有执行finsh_set_prompt_word函数当获取的话,会有问题

如果没有调用了finsh_set_prompt_word这个函数就需要获取提示词的话。在finsh_get_prompt_word这个函数中，会首先判断是否需要提示词模式，如果不需要，则finsh_prompt提示词数组最后一个数据置零后返回。如果需要，因为之前没有调用过finsh_set_prompt_word这个函数，这时的finsh_prompt数组为一个空数组，就会设置默认的提示词同时设置提示词长度。在往后执行，就能获取到finsh_prompt_length变量的长度了。

wdfk-prog · 2025-12-02T00:44:48Z

components/finsh/shell.c

-    /* release semaphore to let finsh thread rx data */
-    rt_sem_release(&shell->rx_sem);
+    if (size)
+        rt_sem_release(&shell->rx_notice);


奇怪的写法

没有size就不释放合适吗

rx_notice为什么要改名字呢

返回值为什么改成0了

1、因为没有数据接收，就不要释放信号量做无效的处理
2、因为改的较多，就把一写变量也就改了，也没其他原因，我拷贝以前的其它代码所以就一并了
3、因为0就是无错误返回，这里等同于返回RT_EOK，这个是习惯问题

wdfk-prog · 2025-12-02T00:45:47Z

components/finsh/shell.c

+    struct rt_device *device;
+    rt_uint16_t oflag;
+    int ret;
+#ifdef RT_USING_SERIAL_V2


这边还要用宏去依赖serial_v2不合适吧?

serial_v2 还是 serial_v1 应该传入同一个参数都能处理来着.兼容的吧?

是兼容，只是想在了解源码的时候，能够知道是适配了串口版本2的

便于后续了解源码的，便利理解

wdfk-prog · 2025-12-02T00:48:46Z

components/finsh/shell.c

+        return;
+
+#ifdef RT_USING_SERIAL_V2
+    oflag = RT_DEVICE_FLAG_RX_BLOCKING | RT_DEVICE_FLAG_TX_BLOCKING;


这里阻塞跟中断没区别吧?

没区别的

Rbb666 · 2025-12-02T02:05:40Z

@youzuwei ci问题需要修正下

Copilot

Pull request overview

This PR performs a major reconstruction of the FinSH shell component, transitioning from array-based to list-based command history management and introducing new snapshot functionality for undo/redo operations. The changes affect core shell behavior, memory management, and user interaction patterns.

Key Changes

List-based history management: Replaces fixed array with dynamic linked list for command history, improving flexibility and memory efficiency
Snapshot feature: Adds Ctrl+Z/Ctrl+Y undo/redo functionality with configurable depth, tracking command editing states
Refactored input handling: State machine approach for processing keyboard inputs (normal, function keys, extended keys, Ctrl sequences)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 21 comments.

File	Description
components/finsh/shell.h	Restructured shell data structures with new enums for input states and snapshot states; changed from array-based to list-based history; updated API signatures to use `rt_bool_t` instead of bitfields; added new public APIs for prompt and device management
components/finsh/shell.c	Complete refactoring of shell implementation: new memory allocation/deallocation for history and snapshots; extracted key handling into separate state-based functions; added snapshot push/pop/rollback logic; improved error handling and documentation; introduced finsh_shell_init/deinit lifecycle management
components/finsh/Kconfig	Added `FINSH_USING_SNAPSHOT` feature with configurable depth; renamed `FINSH_USING_FUNC_EXT` to `FINSH_USING_EXTEND_FEATURE` (breaking change); increased default history lines from 5 to 10; added prompt configuration options; reorganized indentation for better readability

components/finsh/shell.c

components/finsh/Kconfig

components/finsh/shell.c

components/finsh/shell.h

Copilot · 2025-12-02T02:13:52Z

components/finsh/shell.c

+#ifndef RT_USING_HEAP
+    int i;
+    for (i = 0; i < FINSH_SNAPSHOT_DEPTH; i++)
+    {
+        if (shell->snapshot[i].list.next && shell->snapshot[i].list.prev)
+            continue;
+        snap = &shell->snapshot[i];
+        break;
+    }
+    if (i >= FINSH_SNAPSHOT_DEPTH)
+    {
+        rt_kprintf("No available snapshot buffer zone!\n");
+        return RT_NULL;
+    }


Potential bug: Uninitialized variable / 潜在错误：未初始化变量

English: Similar to the history allocation, in the non-heap case, if none of the FINSH_SNAPSHOT_DEPTH slots are available, the variable snap remains uninitialized when reaching the check at line 524. This will cause undefined behavior.

Recommendation: Initialize snap = RT_NULL; before the loop.

中文: 与历史记录分配类似，在非堆情况下，如果所有 FINSH_SNAPSHOT_DEPTH 槽位都不可用，变量 snap 在第 524 行的检查处保持未初始化状态。这将导致未定义行为。

建议: 在循环前初始化 snap = RT_NULL;。

这里不影响，因为如果没有可用的FINSH_SNAPSHOT_DEPTH 槽位，会直接返回RT_NULL

这里也不用更改，因为槽位不可用的时候，将直接返回RT_NULL

Copilot · 2025-12-02T02:13:53Z

components/finsh/shell.h

+int finsh_set_prompt_word(const char *prompt);
+const char *finsh_get_prompt_word(void);
+#define FINSH_PROMPT finsh_get_prompt_word()
+
+void finsh_set_prompt(rt_bool_t prompt);
+rt_bool_t finsh_get_prompt(void);


[nitpick] Inconsistent naming / 命名不一致

English: The function name finsh_set_prompt_word and finsh_get_prompt_word use "word" to indicate they work with the prompt text/string. However, the related functions finsh_set_prompt and finsh_get_prompt work with the prompt mode (boolean). This naming is confusing as "prompt" could refer to either the text or the mode.

Recommendation: Consider renaming to make the distinction clearer:

finsh_set_prompt_text / finsh_get_prompt_text for the string

finsh_set_prompt_enabled / finsh_get_prompt_enabled for the boolean

or keep current naming but ensure it's well documented.

中文: 函数名 finsh_set_prompt_word 和 finsh_get_prompt_word 使用"word"表示它们处理提示文本/字符串。然而，相关函数 finsh_set_prompt 和 finsh_get_prompt 处理提示模式（布尔值）。这种命名令人困惑，因为"prompt"可以指文本或模式。

建议: 考虑重命名以使区别更清晰：

finsh_set_prompt_text / finsh_get_prompt_text 用于字符串

finsh_set_prompt_enabled / finsh_get_prompt_enabled 用于布尔值

或保持当前命名但确保有良好的文档说明。

Suggested change

int finsh_set_prompt_word(const char *prompt);

const char *finsh_get_prompt_word(void);

#define FINSH_PROMPT finsh_get_prompt_word()

void finsh_set_prompt(rt_bool_t prompt);

rt_bool_t finsh_get_prompt(void);

int finsh_set_prompt_text(const char *prompt_text);

const char *finsh_get_prompt_text(void);

#define FINSH_PROMPT finsh_get_prompt_text()

void finsh_set_prompt_enabled(rt_bool_t enabled);

rt_bool_t finsh_get_prompt_enabled(void);

Copilot · 2025-12-02T02:13:53Z

components/finsh/shell.c

+/* clang-format off */
+ #define finsh_printf(fmt, ...) do { if (shell->is_echo) rt_kprintf(fmt, ##__VA_ARGS__); } while (0)
+ #define finsh_puts(str) do { if (shell->is_echo) rt_kputs(str); } while (0)
+ #define finsh_putc(ch) do { if (shell->is_echo) rt_kprintf("%c", ch); } while (0)
+/* clang-format on */


[nitpick] Documentation: Missing comment explanation / 文档：缺少注释说明

English: The macro uses "clang-format off/on" directives but doesn't explain why the macros need special formatting. A comment explaining that these macros use GNU statement expressions (do-while-0 pattern) and need to avoid auto-formatting would be helpful for maintainability.

中文: 宏使用了"clang-format off/on"指令，但没有解释为什么这些宏需要特殊格式化。添加注释说明这些宏使用 GNU 语句表达式（do-while-0 模式）并需要避免自动格式化，这对可维护性有帮助。

Co-authored-by: Copilot <[email protected]>

…tential bugs

…r than or equal' condition

Reconstruct the shell, manage historical commands using list, and add…

c54a634

… the ctrl+z/y functions

github-actions bot added component: finsh Component labels Dec 1, 2025

youzuwei added 2 commits December 1, 2025 21:26

Fix return values in shell.c to use RT_ prefixed error codes and clea…

3bbdd61

…n up comments for consistency.

Refactor shell.c and shell.h to replace NULL with RT_NULL, update siz…

e54ec9b

…e_t to rt_size_t.

wdfk-prog reviewed Dec 2, 2025

View reviewed changes

Rbb666 requested a review from Copilot December 2, 2025 02:05

Copilot started reviewing on behalf of Rbb666 December 2, 2025 02:06 View session

Copilot finished reviewing on behalf of Rbb666 December 2, 2025 02:10

Copilot AI reviewed Dec 2, 2025

View reviewed changes

youzuwei and others added 9 commits December 2, 2025 10:33

Update components/finsh/shell.c

e75b33e

Co-authored-by: Copilot <[email protected]>

Update components/finsh/Kconfig

1ef140c

Co-authored-by: Copilot <[email protected]>

Update components/finsh/shell.c

f390451

Co-authored-by: Copilot <[email protected]>

Update components/finsh/Kconfig

8c371d8

Co-authored-by: Copilot <[email protected]>

Update components/finsh/shell.c

7e75c07

Co-authored-by: Copilot <[email protected]>

Update components/finsh/shell.c

793a78f

Co-authored-by: Copilot <[email protected]>

Update components/finsh/shell.h

c78e433

Co-authored-by: Copilot <[email protected]>

Refactor shell prompt handling and configuration options, fix some po…

b99cde0

…tential bugs

Update command length check in finsh_handle_normal_key to use 'greate…

5d0d6ed

…r than or equal' condition

Reconstruct the shell, manage historical commands using list, and add the ctrl+z/y functions #10986

Are you sure you want to change the base?

Reconstruct the shell, manage historical commands using list, and add the ctrl+z/y functions #10986

Uh oh!

Conversation

youzuwei commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

拉取/合并请求描述：(PR description)

为什么提交这份PR (why to submit this PR)

你的解决方案是什么 (what is your solution)

请提供验证的bsp和config (provide the config and bsp)

当前拉取/合并请求的状态 Intent for your PR

代码质量 Code Quality：

Uh oh!

github-actions bot commented Dec 1, 2025

🛠 操作步骤 | Steps

Uh oh!

github-actions bot commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 Code Review Assignment

🏷️ Tag: components

📊 Current Review Status (Last Updated: 2025-12-02 13:42 CST)

📝 Review Instructions

Uh oh!

wdfk-prog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youzuwei Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rbb666 commented Dec 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

youzuwei commented Dec 1, 2025 •

edited

Loading

github-actions bot commented Dec 1, 2025 •

edited

Loading

youzuwei Dec 2, 2025 •

edited

Loading