Re: [PATCH] poke: Improve .set dot command

[Jose E. Marchesi]

Hi Mohammad.

> Now user can call .set without arguments and get a list of all current
> values.
>
> 2021-12-03  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
>
>       * poke/pk-cmd.c (pk_cmd_exec_1): Introduce a new predicate
>       `run_default_handler_p`. Now if user calls commands (which has
>       sub-commands) with no arguments, it'll try to call the top-level
>       handler (if there's one) instead of showing the usage string.
>       If there's no top-level handler, the usage string will be shown.
>       * poke/pk-cmd-set.c (pk_cmd_set_dump): New function to dump all
>       settings.
>       (set_cmd): Use `pk_cmd_set_dump`, and remove usage string.
>       * poke/pk-settings.pk (pk_settings_dump): New helper function.
> ---
>  ChangeLog           | 12 ++++++++++++
>  poke/pk-cmd-set.c   | 15 ++++++++++++++-
>  poke/pk-cmd.c       | 15 +++++++++++++--
>  poke/pk-settings.pk | 17 +++++++++++++++++
>  4 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 5a1aa21c..cefe7060 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,15 @@
> +2021-12-03  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> +
> +     * poke/pk-cmd.c (pk_cmd_exec_1): Introduce a new predicate
> +     `run_default_handler_p`. Now if user calls commands (which has
> +     sub-commands) with no arguments, it'll try to call the top-level
> +     handler (if there's one) instead of showing the usage string.
> +     If there's no top-level handler, the usage string will be shown.
> +     * poke/pk-cmd-set.c (pk_cmd_set_dump): New function to dump all
> +     settings.
> +     (set_cmd): Use `pk_cmd_set_dump`, and remove usage string.
> +     * poke/pk-settings.pk (pk_settings_dump): New helper function.
> +
>  2021-12-02  Jose E. Marchesi  <jemarch@gnu.org>
>  
>       * libpoke/pkl-trans.c (pkl_trans2_ps_incrdecr): Substitute an
> diff --git a/poke/pk-cmd-set.c b/poke/pk-cmd-set.c
> index 1a471ab2..5d163467 100644
> --- a/poke/pk-cmd-set.c
> +++ b/poke/pk-cmd-set.c
> @@ -26,6 +26,19 @@
>  #include "pk-cmd.h"
>  #include "pk-utils.h"
>  
> +static int
> +pk_cmd_set_dump (int argc, struct pk_cmd_arg argv[], uint64_t uflags)
> +{
> +  pk_val registry_printer, retval;
> +
> +  registry_printer = pk_decl_val (poke_compiler, "pk_settings_dump");
> +  assert (registry_printer != PK_NULL);
> +
> +  if (pk_call (poke_compiler, registry_printer, &retval, 0) == PK_ERROR)
> +    assert (0); /* This shouldn't happen.  */
> +  return 0;
> +}
> +
>  static int
>  pk_cmd_set (int int_p,
>              int argc, struct pk_cmd_arg argv[], uint64_t uflags)
> @@ -261,4 +274,4 @@ pk_cmd_set_init ()
>  struct pk_trie *set_trie;
>  
>  const struct pk_cmd set_cmd =
> -  {"set", "", "", 0, &set_trie, NULL, "set PROPERTY", 
> set_completion_function};
> +  {"set", "", "", 0, &set_trie, pk_cmd_set_dump, "", 
> set_completion_function};
> diff --git a/poke/pk-cmd.c b/poke/pk-cmd.c
> index 77e5c7f6..3c2c6109 100644
> --- a/poke/pk-cmd.c
> +++ b/poke/pk-cmd.c
> @@ -286,6 +286,7 @@ pk_cmd_exec_1 (const char *str, struct pk_trie 
> *cmds_trie, char *prefix)
>    uint64_t uflags;
>    const char *a;
>    int besilent = 0;
> +  int run_default_handler_p = 0;
>  
>    /* Skip blanks, and return if the command is composed by only blank
>       characters.  */
> @@ -350,7 +351,10 @@ pk_cmd_exec_1 (const char *str, struct pk_trie 
> *cmds_trie, char *prefix)
>      {
>        p = skip_blanks (p);
>        if (*p == '\0')
> -        GOTO_USAGE();
> +        {
> +          run_default_handler_p = 1;
> +          GOTO_USAGE();
> +        }
>        return pk_cmd_exec_1 (p, *cmd->subtrie, cmd_name);
>      }
>  
> @@ -549,6 +553,13 @@ pk_cmd_exec_1 (const char *str, struct pk_trie 
> *cmds_trie, char *prefix)
>  
>    besilent = 1;
>    usage:
> +  if (!besilent && run_default_handler_p)
> +    {
> +      if (cmd->handler)
> +        ret = (*cmd->handler) (argc, argv, uflags);
> +      else
> +        run_default_handler_p = 0;
> +    }
>    /* Free arguments occupying memory.  */
>    for (int i = 0; i < argc; ++i)
>      {
> @@ -556,7 +567,7 @@ pk_cmd_exec_1 (const char *str, struct pk_trie 
> *cmds_trie, char *prefix)
>          free (argv[i].val.str);
>      }
>  
> -  if (!besilent)
> +  if (!besilent && !run_default_handler_p)
>      pk_printf (_("Usage: %s\n"), cmd->usage);
>  
>    return ret;
> diff --git a/poke/pk-settings.pk b/poke/pk-settings.pk
> index adfd1a27..ce8a36ed 100644
> --- a/poke/pk-settings.pk
> +++ b/poke/pk-settings.pk
> @@ -390,3 +390,20 @@ pk_help_add_topic
>             summary = "emit errors instead of warnings",
>             description = "",
>      };
> +
> +
> +/* Dump current settings.  */
> +
> +fun pk_settings_dump = void:
> +  {
> +    for (setting in pk_settings.entries)
> +      {
> +        if (setting.kind == POKE_SETTING_INT
> +            || setting.kind == POKE_SETTING_BOOL)
> +          printf ".set %s %i32d\n", setting.name, setting.getter as int;
> +        else if (setting.kind == POKE_SETTING_STR)
> +          printf ".set %s %s\n", setting.name, setting.getter as string;

Why printing .set in each output line?  I think it would be enough to
print something like:

pretty-print 1
obase 16
endian little
auto-map 1
prompt-maps 1
oacutoff 66
odepth 0
oindent 2
omaps 0
omode tree
doc-viewer less

Given that, this is OK for master.
Thanks!

> +        else
> +          assert (0, "uknown kind");
> +      }
> +  }