From 59d52f7fabb48c3a6ebaac869ce3d6e5f3dc704f Mon Sep 17 00:00:00 2001 From: Itamar Haber Date: Mon, 27 Nov 2017 17:57:44 +0200 Subject: [PATCH] Standardizes the 'help' subcommand This adds a new `addReplyHelp` helper that's used by commands when returning a help text. The following commands have been touched: DEBUG, OBJECT, COMMAND, PUBSUB, SCRIPT and SLOWLOG. WIP Fix entry command table entry for OBJECT for HELP option. After #4472 the command may have just 2 arguments. Improve OBJECT HELP descriptions. See #4472. WIP 2 WIP 3 --- src/debug.c | 68 ++++++++++++++++++------------------------------ src/networking.c | 24 +++++++++++++++++ src/object.c | 24 +++++++---------- src/pubsub.c | 16 +++++++++--- src/scripting.c | 17 ++++++++++-- src/server.c | 16 +++++++++--- src/server.h | 1 + src/slowlog.c | 15 ++++++++--- 8 files changed, 112 insertions(+), 69 deletions(-) diff --git a/src/debug.c b/src/debug.c index 5c3fd347..9236d806 100644 --- a/src/debug.c +++ b/src/debug.c @@ -267,48 +267,29 @@ void debugCommand(client *c) { return; } - if (!strcasecmp(c->argv[1]->ptr,"help")) { - void *blenp = addDeferredMultiBulkLength(c); - int blen = 0; - blen++; addReplyStatus(c, - "DEBUG arg arg ... arg. Subcommands:"); - blen++; addReplyStatus(c, - "segfault -- Crash the server with sigsegv."); - blen++; addReplyStatus(c, - "panic -- Crash the server simulating a panic."); - blen++; addReplyStatus(c, - "restart -- Graceful restart: save config, db, restart."); - blen++; addReplyStatus(c, - "crash-and-recovery -- Hard crash and restart after delay."); - blen++; addReplyStatus(c, - "assert -- Crash by assertion failed."); - blen++; addReplyStatus(c, - "reload -- Save the RDB on disk and reload it back in memory."); - blen++; addReplyStatus(c, - "loadaof -- Flush the AOF buffers on disk and reload the AOF in memory."); - blen++; addReplyStatus(c, - "object -- Show low level info about key and associated value."); - blen++; addReplyStatus(c, - "sdslen -- Show low level SDS string info representing key and value."); - blen++; addReplyStatus(c, - "ziplist -- Show low level info about the ziplist encoding."); - blen++; addReplyStatus(c, - "populate [prefix] [size] -- Create string keys named key:. If a prefix is specified is used instead of the 'key' prefix."); - blen++; addReplyStatus(c, - "digest -- Outputs an hex signature representing the current DB content."); - blen++; addReplyStatus(c, - "sleep -- Stop the server for . Decimals allowed."); - blen++; addReplyStatus(c, - "set-active-expire (0|1) -- Setting it to 0 disables expiring keys in background when they are not accessed (otherwise the Redis behavior). Setting it to 1 reenables back the default."); - blen++; addReplyStatus(c, - "lua-always-replicate-commands (0|1) -- Setting it to 1 makes Lua replication defaulting to replicating single commands, without the script having to enable effects replication."); - blen++; addReplyStatus(c, - "error -- Return a Redis protocol error with as message. Useful for clients unit tests to simulate Redis errors."); - blen++; addReplyStatus(c, - "structsize -- Return the size of different Redis core C structures."); - blen++; addReplyStatus(c, - "htstats -- Return hash table statistics of the specified Redis database."); - setDeferredMultiBulkLength(c,blenp,blen); + if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { + const char *help[] = { + "assert -- Crash by assertion failed.", + "crash-and-recovery -- Hard crash and restart after delay.", + "digest -- Outputs an hex signature representing the current DB content.", + "htstats -- Return hash table statistics of the specified Redis database.", + "loadaof -- Flush the AOF buffers on disk and reload the AOF in memory.", + "lua-always-replicate-commands (0|1) -- Setting it to 1 makes Lua replication defaulting to replicating single commands, without the script having to enable effects replication.", + "object -- Show low level info about key and associated value.", + "panic -- Crash the server simulating a panic.", + "populate [prefix] [size] -- Create string keys named key:. If a prefix is specified is used instead of the 'key' prefix.", + "reload -- Save the RDB on disk and reload it back in memory.", + "restart -- Graceful restart: save config, db, restart.", + "sdslen -- Show low level SDS string info representing key and value.", + "segfault -- Crash the server with sigsegv.", + "set-active-expire (0|1) -- Setting it to 0 disables expiring keys in background when they are not accessed (otherwise the Redis behavior). Setting it to 1 reenables back the default.", + "sleep -- Stop the server for . Decimals allowed.", + "structsize -- Return the size of different Redis core C structures.", + "ziplist -- Show low level info about the ziplist encoding.", + "error -- Return a Redis protocol error with as message. Useful for clients unit tests to simulate Redis errors.", + NULL + }; + addReplyHelp(c, help); } else if (!strcasecmp(c->argv[1]->ptr,"segfault")) { *((char*)-1) = 'x'; } else if (!strcasecmp(c->argv[1]->ptr,"panic")) { @@ -550,8 +531,9 @@ void debugCommand(client *c) { addReplyBulkSds(c,stats); } else { - addReplyErrorFormat(c, "Unknown DEBUG subcommand or wrong number of arguments for '%s'", + addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try DEBUG help", (char*)c->argv[1]->ptr); + return; } } diff --git a/src/networking.c b/src/networking.c index aeaeca96..ea8f7d0b 100644 --- a/src/networking.c +++ b/src/networking.c @@ -584,6 +584,30 @@ void addReplyBulkLongLong(client *c, long long ll) { addReplyBulkCBuffer(c,buf,len); } +/* Add an array of strings as a bulk reply with a heading. + * This function is typically invoked by from commands that support + * subcommands in response to the 'help' subcommand. The help array + * is terminated by NULL sentinel. */ +void addReplyHelp(client *c, const char **help) { + sds cmd = sdsnew((char*) c->argv[0]->ptr); + void *blenp = addDeferredMultiBulkLength(c); + int blen = 0; + int hlen = 0; + + sdstoupper(cmd); + addReplyStatusFormat(c, + "%s arg arg ... arg. Subcommands are:",cmd); + blen++; + sdsfree(cmd); + + while (help[hlen]) { + addReplyStatus(c,help[hlen++]); + blen++; + } + + setDeferredMultiBulkLength(c,blenp,blen); +} + /* Copy 'src' client output buffers into 'dst' client output buffers. * The function takes care of freeing the old output buffers of the * destination client. */ diff --git a/src/object.c b/src/object.c index e62b5b3d..2b9403a4 100644 --- a/src/object.c +++ b/src/object.c @@ -1016,20 +1016,15 @@ robj *objectCommandLookupOrReply(client *c, robj *key, robj *reply) { void objectCommand(client *c) { robj *o; - if (!strcasecmp(c->argv[1]->ptr,"help") && c->argc == 2) { - void *blenp = addDeferredMultiBulkLength(c); - int blen = 0; - blen++; addReplyStatus(c, - "OBJECT key. Subcommands:"); - blen++; addReplyStatus(c, - "refcount -- Return the number of references of the value associated with the specified key."); - blen++; addReplyStatus(c, - "encoding -- Return the kind of internal representation used in order to store the value associated with a key."); - blen++; addReplyStatus(c, - "idletime -- Return the number of seconds since the object stored at the specified key is idle."); - blen++; addReplyStatus(c, - "freq -- Return the inverse logarithmic access frequency counter of the object stored at the specified key."); - setDeferredMultiBulkLength(c,blenp,blen); + if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { + const char *help[] = { + "encoding -- Return the kind of internal representation used in order to store the value associated with a key.", + "freq -- Return the access frequency index of the key. The returned integer is proportional to the logarithm of the recent access frequency of the key.", + "idletime -- Return the idle time of the key, that is the approximated number of seconds elapsed since the last access to the key.", + "refcount -- Return the number of references of the value associated with the specified key.", + NULL + }; + addReplyHelp(c, help); } else if (!strcasecmp(c->argv[1]->ptr,"refcount") && c->argc == 3) { if ((o = objectCommandLookupOrReply(c,c->argv[2],shared.nullbulk)) == NULL) return; @@ -1057,6 +1052,7 @@ void objectCommand(client *c) { } else { addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try OBJECT help", (char *)c->argv[1]->ptr); + return; } } diff --git a/src/pubsub.c b/src/pubsub.c index b6d1167d..8bd6e5d6 100644 --- a/src/pubsub.c +++ b/src/pubsub.c @@ -325,8 +325,16 @@ void publishCommand(client *c) { /* PUBSUB command for Pub/Sub introspection. */ void pubsubCommand(client *c) { - if (!strcasecmp(c->argv[1]->ptr,"channels") && - (c->argc == 2 || c->argc ==3)) + if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { + const char *help[] = { + "channels [] -- Return the currently active channels matching a pattern (default: all).", + "numpat -- Return number of subscriptions to patterns.", + "numsub [channel-1 .. channel-N] -- Returns the number of subscribers for the specified channels (excluding patterns, default: none).", + NULL + }; + addReplyHelp(c, help); + } else if (!strcasecmp(c->argv[1]->ptr,"channels") && + (c->argc == 2 || c->argc == 3)) { /* PUBSUB CHANNELS [] */ sds pat = (c->argc == 2) ? NULL : c->argv[2]->ptr; @@ -364,8 +372,8 @@ void pubsubCommand(client *c) { /* PUBSUB NUMPAT */ addReplyLongLong(c,listLength(server.pubsub_patterns)); } else { - addReplyErrorFormat(c, - "Unknown PUBSUB subcommand or wrong number of arguments for '%s'", + addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try PUBSUB help", (char*)c->argv[1]->ptr); + return; } } diff --git a/src/scripting.c b/src/scripting.c index d9f95406..3c6cc078 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -1430,7 +1430,17 @@ void evalShaCommand(client *c) { } void scriptCommand(client *c) { - if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"flush")) { + if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { + const char *help[] = { + "debug (yes|sync|no) -- Set the debug mode for subsequent scripts executed.", + "exists sha1 [sha1 ...] -- Return information about the existence of the scripts in the script cache.", + "flush -- Flush the Lua scripts cache.", + "kill -- Kill the currently executing Lua script.", + "load script -- Load a script into the scripts cache, without executing it.", + NULL + }; + addReplyHelp(c, help); + } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"flush")) { scriptingReset(); addReply(c,shared.ok); replicationScriptCacheFlush(); @@ -1489,9 +1499,12 @@ void scriptCommand(client *c) { c->flags |= CLIENT_LUA_DEBUG_SYNC; } else { addReplyError(c,"Use SCRIPT DEBUG yes/sync/no"); + return; } } else { - addReplyError(c, "Unknown SCRIPT subcommand or wrong # of args."); + addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try SCRIPT help", + (char*)c->argv[1]->ptr); + return; } } diff --git a/src/server.c b/src/server.c index 88892626..457cf8f2 100644 --- a/src/server.c +++ b/src/server.c @@ -276,7 +276,7 @@ struct redisCommand redisCommandTable[] = { {"readonly",readonlyCommand,1,"F",0,NULL,0,0,0,0,0}, {"readwrite",readwriteCommand,1,"F",0,NULL,0,0,0,0,0}, {"dump",dumpCommand,2,"r",0,NULL,1,1,1,0,0}, - {"object",objectCommand,3,"r",0,NULL,2,2,2,0,0}, + {"object",objectCommand,-2,"r",0,NULL,2,2,2,0,0}, {"memory",memoryCommand,-2,"r",0,NULL,0,0,0,0,0}, {"client",clientCommand,-2,"as",0,NULL,0,0,0,0,0}, {"eval",evalCommand,-3,"s",0,evalGetKeys,0,0,0,0,0}, @@ -2733,7 +2733,16 @@ void commandCommand(client *c) { dictIterator *di; dictEntry *de; - if (c->argc == 1) { + if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { + const char *help[] = { + "(no subcommand) -- Return details about all Redis commands.", + "count -- Return the total number of commands in this Redis server.", + "getkeys -- Return the keys from a full Redis command.", + "info [command-name ...] -- Return details about multiple Redis commands.", + NULL + }; + addReplyHelp(c, help); + } else if (c->argc == 1) { addReplyMultiBulkLen(c, dictSize(server.commands)); di = dictGetIterator(server.commands); while ((de = dictNext(di)) != NULL) { @@ -2767,7 +2776,8 @@ void commandCommand(client *c) { for (j = 0; j < numkeys; j++) addReplyBulk(c,c->argv[keys[j]+2]); getKeysFreeResult(keys); } else { - addReplyError(c, "Unknown subcommand or wrong number of arguments."); + addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try COMMAND help", + (char*)c->argv[1]->ptr); return; } } diff --git a/src/server.h b/src/server.h index e3b56075..9b26221f 100644 --- a/src/server.h +++ b/src/server.h @@ -1357,6 +1357,7 @@ void addReplyDouble(client *c, double d); void addReplyHumanLongDouble(client *c, long double d); void addReplyLongLong(client *c, long long ll); void addReplyMultiBulkLen(client *c, long length); +void addReplyHelp(client *c, const char **help); void copyClientOutputBuffer(client *dst, client *src); size_t sdsZmallocSize(sds s); size_t getStringObjectSdsUsedMemory(robj *o); diff --git a/src/slowlog.c b/src/slowlog.c index 32ec4374..9d6f4f02 100644 --- a/src/slowlog.c +++ b/src/slowlog.c @@ -140,7 +140,15 @@ void slowlogReset(void) { /* The SLOWLOG command. Implements all the subcommands needed to handle the * Redis slow log. */ void slowlogCommand(client *c) { - if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"reset")) { + if (!strcasecmp(c->argv[1]->ptr,"help") && c->argc == 2) { + const char *help[] = { + "get [count] -- Return the top entries from the slowlog (default: 10).", + "len -- Return the length of the slowlog.", + "reset -- Reset the slowlog.", + NULL + }; + addReplyHelp(c, help); + } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"reset")) { slowlogReset(); addReply(c,shared.ok); } else if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"len")) { @@ -177,7 +185,8 @@ void slowlogCommand(client *c) { } setDeferredMultiBulkLength(c,totentries,sent); } else { - addReplyError(c, - "Unknown SLOWLOG subcommand or wrong # of args. Try GET, RESET, LEN."); + addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try SLOWLOG help", + (char*)c->argv[1]->ptr); + return; } }