Re: [PATCH 3/3] Build command tries recusively

[Jose E. Marchesi]

    >     +
    >     +      /* if we have subcommands, expand their trie too */
    >     +      if (cmd->subcmds)
    >     +        cmd->subtrie = pk_trie_from_cmds(cmd->subcmds);
    >
    > Please leave a space between the function name and the argument list.
    
    Sorry, force of habit ;)
    
Heh it's the same for me when I write kernel code... it feels weird for
a little while :D

    For Linux we have checkpatch.pl which is a terrible pile of hacks, but
    it does catch most of the common style violations. Is there anything
    similar for projects using the GNU standards?

Not that I'm aware of.  What we have is very picky maintainers for which
it is a kind of a second nature to spot syntax convention violations.

A tool would be much better!

    I tried using GNU indent since that (allegedly) formats to GNU style
    by default, but:
    
    [23:45 oliver ~/.../poke/src ((0f5563d...) %)]$ indent *.[ch]
    indent: pkl-pass.c:396: Error:Unmatched 'else'
    
    I'm not sure what it's complaining about since pkl-pass.c:396 is a
    break statement. and:
    
    [23:46 oliver ~/.../poke/src ((0f5563d...) *%)]$ git diff --shortstat
     41 files changed, 6021 insertions(+), 6141 deletions(-)
    
    So.. I guess GNU indent is not the best thing for the job?

Many of us use Emacs, and it does a good job for indenting and
formatting in the GNU style.  I believe there should be something
similar for vim.

I will look into that indent problem anyway.  Seems like it is getting
confused by the macro hackery of pkl-pass.  It is worth a bug report if
it can be reproduced with the latest version.
    
    >     @@ -779,10 +764,16 @@ pk_cmd_init (void)
    >      void
    >      pk_cmd_shutdown (void)
    >      {
    >     +  struct pk_cmd *cmd;
    >     +  int i;
    >     +
    >     +  for (i = 0, cmd = cmds[0];
    >     +       cmd->name != NULL;
    >     +       cmd = cmds[++i])
    >     +    {
    >     +      if (cmd->subtrie)
    >     +        pk_trie_free (cmd->subtrie);
    >     +    }
    >     +
    >        pk_trie_free (cmds_trie);
    >     -  pk_trie_free (info_trie);
    >     -  pk_trie_free (help_trie);
    >     -  pk_trie_free (vm_trie);
    >     -  pk_trie_free (vm_disas_trie);
    >     -  pk_trie_free (set_trie);
    >      }
    >
    > Hm, but this won't work recursively right?  i.e. what about
    > vm_disas_trie.
    
    It should. The vm_disas_cmd has vm_disas_cmds[] as it's subcommand
    list and the change in pk_trie_from_cmds() willl run recursively if
    the subcommand list is non-null.

Ah ok, I see.