Re: zsh_error_db --- hash-based database of error messages

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Peter Stephenson wrote on Fri, Dec 16, 2022 at 16:42:53 +0000:
> Following on from the retread of the discussion on error messages,
> here's a very simply proof of concept for a hash-based database of
> error messages.  Even if it's adopted I don't intend the C code
> to get much larger as the point is to allow it to be able to do
> everything in shell code.
> 

So, tl;dr:

- Every error message would get an E42 identifier in the source string.

- The "E42" will be looked up as a string key in a well-known assoc,
  where the value will be a more elaborate error message.

- The more elaborate message, if there is one, will be used instead of
  the default message.

Code review below.

> +++ b/Src/utils.c
> @@ -119,6 +119,93 @@ set_widearray(char *mb_array, Widechar_array wca)
> +/* Attempt to use hash zsh_error_db to update message */
> +/**/
> +static const char *
> +zerrmsg_from_hash(const char *msg)
> +{
> +    Param errdb, msgpm;
> +    HashTable errtab;
> +    const char *postcode = msg, *sigmsg, *sigvar, *imsg;
> +    char *errcode, *newmsg;
> +
> +    if (*postcode++ != 'E')
> +	return msg;
> +    while (idigit(*postcode))
> +	++postcode;
> +    if (postcode == msg || *postcode != ':')
> +	return msg;

The first disjunct can't be true at this point, due to the earlier
«*postcode++».  Maybe you meant «postcode == msg + 1», to catch the case
"E:foo" with no digits?

> +
> +    imsg = postcode+1;
> +    errdb = (Param)paramtab->getnode(paramtab, "zsh_error_db");
> +    if (!errdb || !(errdb->node.flags & PM_HASHED)) {
> +	return imsg;
> +    }
> +
> +    errcode = dupstrpfx(msg, postcode-msg);
> +    errtab = errdb->gsu.h->getfn(errdb);
> +    if (!errtab)
> +	return imsg;
> +    msgpm = (Param)errtab->getnode(errtab, errcode);
> +    if (PM_TYPE(msgpm->node.flags)) {
> +	/* Not a plain string, bail out (safety) */
> +	return imsg;
> +    }
> +    newmsg = msgpm->gsu.s->getfn(msgpm);
> +
> +    if (!newmsg || !*newmsg)
> +	return imsg;
> +
> +    /* Check the %-signature matches */
> +    sigmsg = imsg;
> +    sigvar = newmsg;
> +
> +    for (;;) {
> +	while (*sigmsg && *sigmsg != '%')
> +	    sigmsg++;
> +	if (!*sigmsg)
> +	    break;

That's just reinventing strchr(), isn't it?

> +	++sigmsg;
> +	if (*sigmsg == '%') {
> +	    ++sigmsg;
> +	    continue;
> +	}
> +	while (*sigvar) {
> +	    if (*sigvar++ == '%')
> +	    {
> +		if (*sigvar != '%')
> +		    break;
> +		++sigvar;
> +	    }
> +	}
> +	if (!*sigvar || *sigvar != *sigmsg)
> +	    return zerrmsg_bad_signature(errcode, imsg);
> +	++sigvar;
> +	++sigmsg;
> +    }
> +    while (*sigvar)
> +    {
> +	if (*sigvar++ == '%')
> +	{
> +	    if (*sigvar != '%')
> +		return zerrmsg_bad_signature(errcode, imsg);
> +	    ++sigvar;
> +	}
> +    }
> +
> +    return newmsg;
> +}
> +
>  
>  /* Print an error
>