-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[components][finsh] 新增:MSH命令 options补全 #7304
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
Conversation
| { | ||
| switch (MSH_OPT_ID_GET(cmd_list)) | ||
| { | ||
| case RT_Object_Class_Thread: list_thread(); break; |
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.
感觉这么改,有点复杂,还是感觉如何支持二级命令补全。比较方便
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.
这个只是命令函数中的使用示例,补全和这部分代码没关系
|
目前实现的方案是什么,可以在描述里补充一下。 |
将 MSH_CMD_EXPORT_ALIAS 改为可变长度参实现,输入第四个参数时开启选项补全, 将opt的地址放到finsh_syscall中, 输入tab时分析是否需要补全, 通过get到opt的句柄进行二级选项的补全 |
9a257fa to
ae42f73
Compare
ae42f73 to
74e14f3
Compare
|
ROM会额外开销多少? |
aarch64 qemu上 代码量增加2064byte 每个命令增加一个指针, 41个命令 text段增加了328bytes, 随后的大小来自于选项的字符串 |
| case RT_Object_Class_Device: list_device(); break; | ||
| #endif /* RT_USING_DEVICE */ | ||
| #ifdef RT_USING_DFS | ||
| case 0x100: |
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.
直接十六进制数字,什么情况哈
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.
可以考虑另外定义一组宏定义,例如
#define CMD_LIST_OPT_INDEX_THREAD 0
// .....
components/finsh/finsh.h
Outdated
| #endif /* end of FINSH_USING_SYMTAB */ | ||
|
|
||
| #define GET_MACRO(_1, _2, _3, _FUN, ...) _FUN | ||
| #define GET_EXPORT_MACRO(_1, _2, _3, _4, _FUN, ...) _FUN |
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.
这个后续如果有宏名称冲突怎么办?
| rt_kprintf("\n"); | ||
| msh_auto_complete(prefix); | ||
|
|
||
| msh_opt_auto_complete(prefix); |
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.
是否需要和宏FINSH_OPTION_COMPLETION_ENABLED相关?
|
确实是一种模式,不过要把代码整理好,写好,还得多考虑下,加油 |
|
静态代码检查貌似会失败在宏的可变长度参哪里, 不支持这个检测吗? |
| # CONFIG_PKG_USING_MICRO_ROS is not set | ||
| # CONFIG_PKG_USING_MCP23008 is not set | ||
| # CONFIG_PKG_USING_BLUETRUM_SDK is not set | ||
| # CONFIG_PKG_USING_MISAKA_AT24CXX is not set |
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.
這個文件自動生成的,可以刪除。不需要修改。
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.
修改默认值的时候,最好还是确保所有 bsp 都改了吧。不过讲真,个人认为最好直接不要在 git 仓库存这种可以自动生成的文件。linux 就是这样做的。
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.
这文件是存储的用户配置,比如一些选项打开或者没打开,必须要存的,linux 也有存的
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.
这文件是存储的用户配置,比如一些选项打开或者没打开,必须要存的
首先这些文件是可以自动生成的 (kconfig 文件有默认配置值)。其次,维护负担问题,例如修改某个组件的kconfig ,那几乎所有 bsp 的 .config 文件都需要修改 (如果不改, 保持历史状态,存的意义何在?). 最直观的感受是这些 .config 文件都很相似。。。
linux 也有存的
linux 没存,.config 在linux默认是ignore的,最多只有一个 xxx_defaultconfig 文件, 用来存储某个 soc 的部分特殊配置. 然后 ACH=XXX ... make xxx_defaultconfig 自动生成完整的 .config 文件。

| /* #define MSH_CMD_EXPORT_ALIAS(command, alias, desc) or | ||
| #define MSH_CMD_EXPORT_ALIAS(command, alias, desc, opt) */ | ||
| #ifdef FINSH_OPTION_COMPLETION_ENABLED | ||
| #define MSH_CMD_EXPORT_ALIAS(...) \ |
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.
好高端的写法 👍
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
新增:MSH命令 options补全
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality:
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up