Re: [PATCH 1/3] Print subcommands as a part of the usage

[Jose E. Marchesi]

Hello Oliver.

    This is helpful since there's no real way to discover what
    subcommands a command has. Generally the usage information for the
    "root" of a subcommand set isn't terribly useful anyway, so I think
    this is an improvement.
    
    Case in point, the usage information for .info is wrong since
    there's no "type" subcommand:
    
    (poke) .info
    Usage: info (files|variable|type)
    subcommands:
          files - info files
          variable - info variable
          function - info funtion
    (poke)
    
    Those usage strings will probably need fixing, but I figured
    I'd see what people thought of the approach first.

I love it.  It is certainly much better to have the subcommands in the
definition of the command itself, than what I implemented initially :)

Regarding the usage strings for top commands, I think we should change
them to refer to abstract entities, and let the subcommands to provide
the concrete options that are accepted.  Like e.g.:

{"disassemble", "e", PK_VM_DIS_UFLAGS, 0, &vm_disas_trie, NULL,
  "vm disassemble WHAT", &vm_disas_trie}

    Signed-off-by: Oliver O'Halloran <address@hidden>
    ---
     src/pk-cmd.c  | 11 ++++++++++-
     src/pk-cmd.h  |  5 ++++-
     src/pk-help.c |  2 +-
     src/pk-info.c |  2 +-
     src/pk-set.c  |  2 +-
     src/pk-vm.c   |  6 +++---
     6 files changed, 20 insertions(+), 8 deletions(-)
    
    diff --git a/src/pk-cmd.c b/src/pk-cmd.c
    index 680d5f6..31b990c 100644
    --- a/src/pk-cmd.c
    +++ b/src/pk-cmd.c
    @@ -569,8 +569,17 @@ pk_cmd_exec_1 (char *str, struct pk_trie *cmds_trie, 
char *prefix)
       return ret;
     
      usage:
    -  if (!besilent)
    +  if (!besilent) {

Please place open braces in their own line, like specified by the GNU
coding standards.

Other than that, the patch looks OK to me! :)

         printf (_("Usage: %s\n"), cmd->usage);
    +    if (cmd->subtrie) {
    +      printf (_("subcommands:\n"));
    +      for (i = 0; cmd->subcmds[i] != &null_cmd; i++)
    +        {
    +          printf (_("      %s - %s\n"),
    +                  cmd->subcmds[i]->name, cmd->subcmds[i]->usage);
    +        }
    +    }
    +  }
       return 0;
     }
     
    diff --git a/src/pk-cmd.h b/src/pk-cmd.h
    index 640e2ca..f498ce2 100644
    --- a/src/pk-cmd.h
    +++ b/src/pk-cmd.h
    @@ -75,11 +75,14 @@ struct pk_cmd
       /* A value composed of or-ed PK_CMD_F_* flags.  See above.  */
       int flags;
       /* Subcommands.  */
    -  struct pk_trie **subtrie;
    +  struct pk_cmd **subcmds;
       /* Function implementing the command.  */
       pk_cmd_fn handler;
       /* Usage message.  */
       const char *usage;
    +
    +  /* tail element that we don't need to initialise */
    +  struct pk_trie **subtrie;
     };
     
     /* Parse STR and execute a command.  */
    diff --git a/src/pk-help.c b/src/pk-help.c
    index 017bbc2..54d018e 100644
    --- a/src/pk-help.c
    +++ b/src/pk-help.c
    @@ -29,4 +29,4 @@ struct pk_cmd *help_cmds[] =
     struct pk_trie *help_trie;
     
     struct pk_cmd help_cmd =
    -  {"help", "", "", 0, &help_trie, NULL, "help COMMAND"};
    +  {"help", "", "", 0, help_cmds, NULL, "help COMMAND", &help_trie};
    diff --git a/src/pk-info.c b/src/pk-info.c
    index b8e7f03..28f88b7 100644
    --- a/src/pk-info.c
    +++ b/src/pk-info.c
    @@ -35,4 +35,4 @@ struct pk_cmd *info_cmds[] =
     struct pk_trie *info_trie;
     
     struct pk_cmd info_cmd =
    -  {"info", "", "", 0, &info_trie, NULL, "info (files|variable|type)"};
    +  {"info", "", "", 0, info_cmds, NULL, "info (files|variable|type)", 
&info_trie};
    diff --git a/src/pk-set.c b/src/pk-set.c
    index 0364763..354d1d3 100644
    --- a/src/pk-set.c
    +++ b/src/pk-set.c
    @@ -197,4 +197,4 @@ struct pk_cmd *set_cmds[] =
     struct pk_trie *set_trie;
     
     struct pk_cmd set_cmd =
    -  {"set", "", "", 0, &set_trie, NULL, "set PROPERTY"};
    +  {"set", "", "", 0, set_cmds, NULL, "set PROPERTY", &set_trie};
    diff --git a/src/pk-vm.c b/src/pk-vm.c
    index 67569e4..b810572 100644
    --- a/src/pk-vm.c
    +++ b/src/pk-vm.c
    @@ -201,8 +201,8 @@ struct pk_cmd *vm_disas_cmds[] =
     struct pk_trie *vm_disas_trie;
     
     struct pk_cmd vm_disas_cmd =
    -  {"disassemble", "e", PK_VM_DIS_UFLAGS, 0, &vm_disas_trie, NULL,
    -   "vm disassemble (expression|function)"};
    +  {"disassemble", "e", PK_VM_DIS_UFLAGS, 0, vm_disas_cmds, NULL,
    +   "vm disassemble (expression|function)", &vm_disas_trie};
     
     struct pk_cmd *vm_cmds[] =
       {
    @@ -213,4 +213,4 @@ struct pk_cmd *vm_cmds[] =
     struct pk_trie *vm_trie;
     
     struct pk_cmd vm_cmd =
    -  {"vm", "", "", 0, &vm_trie, NULL, "vm (disassemble)"};
    +  {"vm", "", "", 0, vm_cmds, NULL, "vm (disassemble)", &vm_trie};