[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] Make readline completers a little more useful

From: Jose E. Marchesi
Subject: Re: [PATCH 2/2] Make readline completers a little more useful
Date: Thu, 14 Nov 2019 18:38:51 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Hi John.
    diff --git a/src/pk-repl.c b/src/pk-repl.c
    index a81e371..3883686 100644
    --- a/src/pk-repl.c
    +++ b/src/pk-repl.c
    @@ -35,6 +35,65 @@
     #include <signal.h>
     #include <unistd.h>
    +extern struct pk_cmd null_cmd;

Is this extern still necessary?

    +static char *
    +poke_completion_function (const char *x, int state)
    +  static int idx = 0;
    +  static struct pkl_ast_node_iter iter;
    +  pkl_env env = pkl_get_env (poke_compiler);
    +  if (state == 0)
    +    {
    +      pkl_env_iter_begin (env, &iter);
    +      idx = 0;
    +    }
    +  else
    +    {
    +      if (pkl_env_iter_end (env, &iter))
    +   idx++;
    +      else
    +   pkl_env_iter_next (env, &iter);
    +    }
    +  size_t len = strlen (x);
    +  char *function_name;
    +  function_name = get_next_matching_cmd (env, &iter, x, len);
    +  if (function_name)
    +    return function_name;
    +  function_name = dot_command_completer_next (&idx, x, len);
    +  if (function_name)
    +    return function_name;
    +  return NULL;

Much better.  Now the poke_completion_function limits itself to use the
PKL env iterators.

    +/*  Return the name of the next command that are currently
    +    in context of ENV and match NAME,LEN.  ITER is an iterator
    +    into the set of matches.  Returns the name of the next
    +    command in the set, or NULL if there are no more.
    +    The returned value must be freed by the caller.  */
    +char *
    +get_next_matching_cmd (pkl_env env, struct pkl_ast_node_iter *iter,
    +                  const char *name, size_t len)

I would change the name of the function to:

1) be coherent with the rest of public functions, i.e. to use a pkl_env_

2) I would avoid talking about "commands" in the PKL context.  From the
   compiler perspective there are no "commands".  There are
   declarations.  But since the PKL environment iterators iterate over
   declarations (which are the entities composed the environment) I see
   little value of reiterate it in the function's name.

3) The function works on an iterator, so why not having it as part of
   the env iterator API?

So what about just calling it pkl_env_iter_match?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]