Re: [chrony-dev] [PATCH 1/1] client: re-work tab-completion to work with libedit 20180525-3.1

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Wed, Jun 13, 2018 at 02:59:56PM -0500, Lonnie Abelbeck wrote:
> The key was to use the readline/libedit global "rl_line_buffer" to look-back from "start" to determine which command options should be matched, if any.
> 
I like that the the list of commands doesn't have duplicates with and
without the -v option.

Few comments about the patch:

> +  const char *name, *names[5][64] = {

Could you please consider using an array of pointers to arrays instead
of the two-dimensional array? I don't like that there is a wasted space
and the maximum length has to specified. I think you could use
separate arrays for the base commands, add commands, and so on, and
then point to them in the names array.

> @@ -1317,7 +1325,26 @@ command_name_generator(const char *text, int state)
>  static char **
>  command_name_completion(const char *text, int start, int end)
>  {
> +  char first[32];
> +  int cnt = start > sizeof(first)-1 ? sizeof(first)-1 : start;

Please use the MIN macro here.

> +
> +  strncpy(first, rl_line_buffer, cnt);
> +  first[cnt] = '\0';

And snprintf instead of strncpy.

Thanks,

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.