add command docs impl

[tedli]

impl of #1592

irrelevant changes are made by clang-format.

[cr-gpt]

The code patch appears to add a new command named "CommandCmd" with the kCmdNameCommand identifier. It also modifies the RPopLPushCmd function by replacing the line break with indentation. No bug risk is present, but from the limited information provided, some potential improvement suggestions include using emplace instead of insert for performance and readability when initializing a map. Additionally, it may be helpful to provide more context on the structure and purpose of the code snippet for better review accuracy.

[tedli]

pika_command_docs.cc was generated by a c script:

Click to view

void printDocFlagForCommand(uint64_t flags, replyFlagNames *replyFlags, const char* prefix) {
    int count = 0, j=0;
    /* Count them so we don't have to use deferred reply. */
    while (replyFlags[j].name) {
        if (flags & replyFlags[j].flag)
            count++;
        j++;
    }

    // addReplySetLen(c, count);
    j = 0;
    while (replyFlags[j].name) {
        if (flags & replyFlags[j].flag)
            printf("%s\t\t\"%s\"_RedisStatus,\n", prefix, replyFlags[j].name);
            // addReplyStatus(c, replyFlags[j].name);
        j++;
    }
}

void printDocFlagsForCommand(struct redisCommand *cmd, const char* prefix) {
    replyFlagNames docFlagNames[] = {
        {CMD_DOC_DEPRECATED,         "deprecated"},
        {CMD_DOC_SYSCMD,             "syscmd"},
        {0,NULL}
    };
    printDocFlagForCommand(cmd->doc_flags, docFlagNames, prefix);
}

void printReplyFlagsForArg(uint64_t flags, const char* prefix) {
    replyFlagNames argFlagNames[] = {
        {CMD_ARG_OPTIONAL,          "optional"},
        {CMD_ARG_MULTIPLE,          "multiple"},
        {CMD_ARG_MULTIPLE_TOKEN,    "multiple_token"},
        {0,NULL}
    };
    printDocFlagForCommand(flags, argFlagNames, prefix);
}

void printCommandArgList(struct redisCommandArg *args, int num_args, const char* prefix) {
    // addReplyArrayLen(c, num_args);
    for (int j = 0; j<num_args; j++) {
        /* Count our reply len so we don't have to use deferred reply. */
        int has_display_text = 1;
        long maplen = 2;
        if (args[j].key_spec_index != -1) maplen++;
        if (args[j].token) maplen++;
        if (args[j].summary) maplen++;
        if (args[j].since) maplen++;
        if (args[j].deprecated_since) maplen++;
        if (args[j].flags) maplen++;
        if (args[j].type == ARG_TYPE_ONEOF || args[j].type == ARG_TYPE_BLOCK) {
            has_display_text = 0;
            maplen++;
        }
        if (has_display_text) maplen++;
        // addReplyMapLen(c, maplen);
        printf("%s\t\tRedisMap({\n", prefix);

        // addReplyBulkCString(c, "name");
        // addReplyBulkCString(c, args[j].name);
        printf("%s\t\t\t{\"name\", \"%s\"_RedisString},\n", prefix, args[j].name);

        // addReplyBulkCString(c, "type");
        // addReplyBulkCString(c, ARG_TYPE_STR[args[j].type]);
        printf("%s\t\t\t{\"type\", \"%s\"_RedisString},\n", prefix, ARG_TYPE_STR[args[j].type]);

        if (has_display_text) {
            // addReplyBulkCString(c, "display_text");
            // addReplyBulkCString(c, args[j].display_text ? args[j].display_text : args[j].name);
            printf("%s\t\t\t{\"display_text\", \"%s\"_RedisString},\n",
             prefix, args[j].display_text ? args[j].display_text : args[j].name);
        }
        if (args[j].key_spec_index != -1) {
            // addReplyBulkCString(c, "key_spec_index");
            // addReplyLongLong(c, args[j].key_spec_index);
            printf("%s\t\t\t{\"key_spec_index\", %d_RedisInt},\n", prefix, args[j].key_spec_index);
        }
        if (args[j].token) {
            // addReplyBulkCString(c, "token");
            // addReplyBulkCString(c, args[j].token);
            printf("%s\t\t\t{\"token\", \"%s\"_RedisString},\n", prefix, args[j].token);
        }
        if (args[j].summary) {
            // addReplyBulkCString(c, "summary");
            // addReplyBulkCString(c, args[j].summary);
            printf("%s\t\t\t{\"summary\", \"%s\"_RedisString},\n", prefix, args[j].summary);
        }
        if (args[j].since) {
            // addReplyBulkCString(c, "since");
            // addReplyBulkCString(c, args[j].since);
            printf("%s\t\t\t{\"since\", \"%s\"_RedisString},\n", prefix, args[j].since);
        }
        if (args[j].deprecated_since) {
            // addReplyBulkCString(c, "deprecated_since");
            // addReplyBulkCString(c, args[j].deprecated_since);
            printf("%s\t\t\t{\"deprecated_since\", \"%s\"_RedisString},\n", prefix, args[j].deprecated_since);
        }
        if (args[j].flags) {
            // addReplyBulkCString(c, "flags");
            // addReplyFlagsForArg(c, args[j].flags);
            char buffer[32] = {0};
            redis_strlcat(buffer, prefix, 32);
            redis_strlcat(buffer, "\t\t", 32);
            printf("%s\t\t\t{\"flags\", RedisArray({\n", prefix);
            printReplyFlagsForArg(args[j].flags, buffer);
            printf("%s\t\t\t})},\n", prefix);
        }
        if (args[j].type == ARG_TYPE_ONEOF || args[j].type == ARG_TYPE_BLOCK) {
            // addReplyBulkCString(c, "arguments");
            // addReplyCommandArgList(c, args[j].subargs, args[j].num_args);
            char buffer[32] = {0};
            redis_strlcat(buffer, prefix, 32);
            redis_strlcat(buffer, "\t\t", 32);
            printf("%s\t\t\t{\"arguments\", RedisArray({\n", prefix);
            printCommandArgList(args[j].subargs, args[j].num_args, buffer);
            printf("%s\t\t\t})},\n", prefix);
        }
        printf("%s\t\t}),\n", prefix);
    }
}

void printCommandDocs(struct redisCommand *cmd, const char* prefix) {
    printf("%s{\"%s\", RedisMap({\n", prefix, cmd->fullname);
    if (cmd->summary) {
        printf("%s\t{\"summary\", \"%s\"_RedisString},\n", prefix, cmd->summary);
    }
    if (cmd->since) {
        printf("%s\t{\"since\", \"%s\"_RedisString},\n", prefix, cmd->since);
    }
    printf("%s\t{\"group\", \"%s\"_RedisString},\n", prefix, commandGroupStr(cmd->group));

    if (cmd->complexity) {
        printf("%s\t{\"complexity\", \"%s\"_RedisString},\n", prefix, cmd->complexity);
    }
    if (cmd->flags & CMD_MODULE) {
        printf("%s\t{\"module\", \"%s\"_RedisString},\n", prefix, moduleNameFromCommand(cmd));
    }
    if (cmd->doc_flags) {
        printf("%s\t{\"doc_flags\", RedisSet({\n", prefix);
        printDocFlagsForCommand(cmd, prefix);
        printf("\t})},\n");
    }
    if (cmd->deprecated_since) {
        printf("%s\t{\"deprecated_since\", \"%s\"_RedisString},\n", prefix, cmd->deprecated_since);
    }
    if (cmd->replaced_by) {
        printf("%s\t{\"replaced_by\", \"%s\"_RedisString},\n", prefix, cmd->replaced_by);
    }
    if (cmd->history) {
        printf("%s\t{\"history\", RedisSet({\n", prefix);
        for (int i = 0; i < cmd->num_history; ++i)
            printf("%s\t\tRedisArray({\"%s\"_RedisString, \"%s\"_RedisString}),\n",
             prefix, cmd->history[i].since, cmd->history[i].changes);
        printf("%s\t})},\n", prefix);
    }
    if (cmd->args) {
        printf("%s\t{\"arguments\", RedisArray({\n", prefix);
        printCommandArgList(cmd->args, cmd->num_args, prefix);
        printf("%s\t})},\n", prefix);
    }
    if (cmd->subcommands_dict) {
        char buffer[32] = {0};
            redis_strlcat(buffer, prefix, 32);
            redis_strlcat(buffer, "\t\t", 32);
            printf("%s\t{\"subcommands\", RedisMap({\n", prefix);
            dictEntry *de;
            dictIterator *di = dictGetSafeIterator(cmd->subcommands_dict);
            while((de = dictNext(di)) != NULL) {
                struct redisCommand *sub = (struct redisCommand *)dictGetVal(de);
                printCommandDocs(sub, buffer);
            }
            dictReleaseIterator(di);
            printf("%s\t})},\n", prefix);
    }
    printf("%s})},\n", prefix);
}

[tedli]

Demo

[cr-gpt]

Overall, the code patch seems to be very simple and straightforward. The only change made is adding a new item to the "all_tests" list. As long as the new test file "unit/command" exists and works as intended, there should not be any bugs or risks associated with this change.

One possible improvement suggestion would be to provide more information about what the new test "unit/command" does or tests for. This could help others understand the purpose of the test and why it was added to the list.

Regarding the comment in the code, it is unclear what exactly it is referring to. It may be helpful to provide more context or clarification for this comment.

[cr-gpt]

The code patch seems to be testing whether the "set" command documentation is correctly supported. A few points to note:

• The code seems to be written in Tcl.
• There are no syntax errors or bugs in this code.
• However, it is not clear what the "r" variable refers to and how it is initialized. This could potentially cause issues if the variable is not properly set.
• It may be helpful to add a comment explaining what the "assert" function does and what the expected output of this test is.
• It may also be helpful to use more descriptive names for the test and variables to make the code more readable and maintainable.
• Finally, there is no clear indication of what specific improvements need to be made to this code.

[luky116]

这个空格保留吧，其他地方也是一样的

[tedli]

这里是 clang-format 给格式化的。
正常空实现函数 {} 后面不用再跟分号 ; 了。我试了一下，如果去掉分号，格式化后，不会去掉花括号前面的空格。

override {}; -->override{};  // 花括号前面被去掉了
override {} --> override {}  // 花括号前面空格不会被去掉，甚至如果自己手动去掉的，格式化后，还会被加上空格

[chejinge]

这个保留原来的格式吧，辛苦回退下

[tedli]

这里是 clang-format 给格式化的。

[tedli]

Hi @luky116 @chejinge ，

我可以把本次提交不相关改动（格式化相关的）给回滚回来。可以商量一下，要怎么改，或者保持现状。

其中空函数后面跟分号，我查到 clang format 有个自动去掉冗余分号的规则。

https://clang.llvm.org/docs/ClangFormatStyleOptions.html#removesemicolon

[cr-gpt]

Based on the code patch you provided, here are a few observations and suggestions:

1. Naming Convention: The variable names like kCmdNamePKPatternMatchDel and kCmdNameSlotsMgrtSlot seem quite long and may be difficult to read and maintain. Consider using more concise and descriptive names.

2. Sorting Constants: It's good practice to sort constants alphabetically as it makes them easier to locate and maintain. Make sure to sort all the command constants, including the newly added kCmdNameCommand.

3. Commenting: Consider adding comments to describe the purpose or functionality of certain parts of the code to improve code readability and maintainability.

4. Error Handling: There doesn't appear to be any error handling or validation for the input commands. It would be beneficial to include error checking mechanisms if required, such as ensuring that the command is valid before processing it.

5. Code Structure: It's difficult to fully evaluate the code without seeing the entire context. Consider providing more information or additional code if you have specific concerns or areas you'd like to improve.

Remember that this review is based solely on the provided code patch, and there might be other aspects of your software that need attention but aren't visible here.

[cr-gpt]

Here are some suggestions for the code review:

1. In lines 138, 162, and 217, consider using a const auto& reference instead of const auto& i or const auto& iter. This improves code readability.

2. In line 138, cmd_size += 1 + i.size() can be improved by using cmd_size += 1 + i.size(); without any issues.

3. In lines 162 and 199, consider using += pstd::ToRead(iter) instead of " " + pstd::ToRead(iter). It avoids unnecessary string construction and improves efficiency.

4. In lines 217 and 238, it's recommended to avoid unnecessary line breaks in the code for better readability. Both statements can be written in a single line.

Overall, the code seems fine with no apparent bug risks. The improvements suggested above are mostly related to code readability and minor optimizations.

[cr-gpt]

The code patch you provided appears to be an addition of a new constant string variable called kCmdNameCommand. Here are some observations and suggestions for improvement:

1. Naming Convention: It's generally recommended to follow a consistent naming convention for variables. The existing variables are using a camelCase naming convention, so it would be better to name the new variable as kCmdNameCommand (as you have done) to maintain consistency.

2. Comment: It would be good practice to add a comment explaining the purpose or usage of the new constant (kCmdNameCommand) if it's not already clear from the context. This can help other developers understand the code.

3. Organization: Consider keeping the constants in alphabetical order to improve readability and maintain a consistent style. You can place kCmdNameCommand in the appropriate position based on its alphabetical order with respect to the other constants.

4. Error Handling: Depending on how these constant strings are used, ensure that error handling is implemented appropriately when dealing with user input or command processing. This could involve checking for valid input, handling unexpected cases, and providing meaningful error messages.

5. Additional Review: It's difficult to provide a comprehensive code review without the full context of the codebase. Other factors like coding conventions, adherence to design patterns, encapsulation, and error handling should also be considered during a thorough code review.

Remember, code reviews benefit from multiple perspectives, so it's always valuable to have your peers or experienced developers review your code as well.

[yaoyinnan]

目前该PR是将 redis COMMAND DOCS 命令支持了，但是显示的还是 redis支持的命令。
pika有一部分命令完全兼容，在这个文档里：https://github.com/OpenAtomFoundation/pika/wiki/pika-%E6%94%AF%E6%8C%81%E7%9A%84redis%E6%8E%A5%E5%8F%A3%E5%8F%8A%E5%85%BC%E5%AE%B9%E6%83%85%E5%86%B5
还有一部分命令无法完全兼容，有差异化，需要等我们测试确定后补充上去。
你可以试试看先把已经完全兼容的命令改上去，这个PR的预期结果就是输出已经完全兼容的命令就可以了。

[yaoyinnan]

这文档包含差异化命令：https://github.com/OpenAtomFoundation/pika/wiki/pika-%E5%B7%AE%E5%BC%82%E5%8C%96%E5%91%BD%E4%BB%A4。
我们目前还在测试，你可以对照参考。

这是全部命令，包含全部兼容的和差异化的：https://github.com/OpenAtomFoundation/pika/blob/unstable/include/pika_command.h

[lqxhub]

定义模板的时候 好像更推荐使用 typename 这个关键字

[tedli]

typename 功能更大，这里只是想限制一下类型能力。已经改成 typename 了。

[cr-gpt]

Overall, the code patch seems to introduce a few changes related to the PikaCmdTableManager class. Here's an overview of the changes:

1. The destructor for PikaCmdTableManager has been modified from a virtual non-default destructor (virtual ~PikaCmdTableManager(){}) to a default virtual destructor (virtual ~PikaCmdTableManager() = default;). This change utilizes the default destructor implementation provided by the compiler.

2. A new member function bool CmdExist(const std::string& cmd) const; has been added to the class. This function is used to check if a command exists.

3. The existing member function std::shared_ptr<Cmd> NewCommand(const std::string& opt); remains without any apparent changes.

Based on this information, here are some observations and suggestions for improvement:

1. Since the destructor was changed to a default virtual destructor, you should ensure that there are no explicit calls to delete objects of PikaCmdTableManager through a pointer to the base class. If you don't need polymorphic behavior or subclasses derived from PikaCmdTableManager, making the destructor non-virtual may be more appropriate.

2. It's difficult to provide detailed feedback or identify potential issues without being able to see the full code context (e.g., definition and usage of Cmd class). Ensure that the class's implementation aligns with its intended functionality.

3. Consider adding proper error handling and validation mechanisms to GetCmd and DistributeKey member functions to handle exceptional cases or invalid inputs. It's important to ensure that these functions behave predictably and are resilient to unexpected scenarios.

4. Verify that the addition of CmdExist doesn't introduce unnecessary overhead or redundancy. Ensure that it provides value and is used appropriately in your codebase.

Remember that a thorough code review would require an understanding of the complete codebase, the surrounding context, and the requirements it serves. These suggestions are based solely on the information provided in the code patch you provided.

[cr-gpt]

The provided code patch appears to be a section of code related to command handling in a project. Here are some suggestions for improvement:

1. Header file inclusion:

   • Headers <memory>, <unordered_set>, and <utility> are included but not used in the code. You can remove them to reduce unnecessary dependencies.

2. Formatting:

   • The comments in the code have inconsistent indentation. Ensure consistent formatting throughout the codebase.

3. Variable naming:

   • The variable names could be more descriptive. For example, _slot and _sync_slot in the ProcessArg constructor could be renamed to better reflect their purpose.

4. Class member initialization:

   • In the Cmd class, consider using initializer lists to initialize members in the constructor instead of assignments in the body of the constructor.

5. Remove unused functions:

   • The function current_key() is declared but not defined in the code. Remove it if it's not being used.

6. Consider refactoring duplicated code:

   • There are instances of repeated code, such as the assignment operator and the copy constructor of the Cmd class. Instead of marking them as private and not implementing them, you can use the default keyword to let the compiler generate them automatically.

7. Documentation:

   • Add comments, especially for complex logic or non-obvious functionality, to improve code readability and maintainability.

These suggestions aim to enhance the code quality, but ultimately, the specific improvements depend on the context and requirements of your project.

[tedli]

redis-cli 正常不显示中文，需要加 --raw，不过一 raw 了显示的缩进格式也没了。