feat:Subcommands support separate command id #24
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
panlei-coder
reviewed
Nov 1, 2023
src/client.cc
Outdated
| return 0; | ||
| } | ||
|
|
||
| if (!cmdPtr->CheckArg(params.size())) { | ||
| ReplyError(PError_param, &reply_); | ||
| if (cmdPtr.first->CheckArg(params.size())) { |
There was a problem hiding this comment.
这里应该是if (!cmdPtr.first->CheckArg(params.size())) ，我在本地输入命令报错
Centurybbx
reviewed
Nov 1, 2023
| if (ctx.argv_.size() < 2) { | ||
| return std::pair(nullptr, CmdRes::kInvalidParameter); | ||
| } | ||
| return std::pair(cmd->second->GetSubCmd(ctx.argv_[1]), CmdRes::kSyntaxErr); |
There was a problem hiding this comment.
这里没太明白逻辑，当没有subcommand时，状态应该返回kSyntaxErr吗？
There was a problem hiding this comment.
在调用GetCommand的地方会判断,如果有子命令,但是返回的是null, 状态是kSyntaxErr, 如果返回的指针不是null, 这个状态会被忽略
src/client.cc
Outdated
| return 0; | ||
| } | ||
|
|
||
| if (!cmdPtr->CheckArg(params.size())) { | ||
| ReplyError(PError_param, &reply_); | ||
| if (cmdPtr->CheckArg(params.size())) { |
There was a problem hiding this comment.
Suggested change
| if (cmdPtr->CheckArg(params.size())) { | |
| if (!cmdPtr->CheckArg(params.size())) { |
这里的判断条件应该是反了
Centurybbx
approved these changes
Nov 2, 2023
panlei-coder
approved these changes
Nov 2, 2023
AlexStocks
approved these changes
Nov 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
每个拥有子命令的命令 (比如 config get, config set) 这样的命令, 以前的时候是一个
config命令, 通过 第二个参数来区分具体执行哪个,config set,config get这些命令id是同一个.这样会带来一个问题,
config set在ACL中属于写命令,config get在ACL中属于 读命令, 如果使用 之前那种所有子命令id相同的时, 无法为子命令区分 不同的ACL类型现在改成了 每个子命令(和redis 处理方式一样) 有单独的id, 可以为不同的子命令 区分不同的 ACL类型, 方便 权限划分
std::pair<BaseCmd*, CmdRes::CmdRet> CmdTableManager::GetCommand(const std::string& cmdName, CmdContext& ctx)这里的返回值使用了
std::pair, 来同时返回两个值, 不知出否妥当, 如有更好的解决方法,还请指正