Subject: bug#63251: AW: bug#63251: 28.2; vhdl-mode contribution

From: Cyril Arnould <[email-masked]>
Date: Sun, 7 May 2023 15:40:12 +0000

Thanks for the feedback.

> There is (cons ...) which would be more precise, see the manual.

I had tried (cons …) instead of (sexp …), but that just resulted in
the customization menu breaking again if one of the compilers was set
to a dotted list.

> The new doc string says that a TYPE of 2 is allowed but the type spec doesn't allow it.
> Either allow both 2 and nil or change the docs to only mention one of them.

Makes sense. It’s probably more user-friendly (not to mention easier)
to just allow one of them.

> Think of what happens if later code performs an in-place change of that nil you added.

I am by no means an expert when it comes to elisp, I don’t know what
kind of problems this could cause. Would using 2 rather than nil make
more sense for this? I’ve checked compile.el, and internally they
remap nil to 2 and use that, so 2 would also be more explicit I guess.

I've modifided the patch to use 2 rather than nil (exclusively).

Von: Mattias Engdegård<mailto:[email-masked]>
Gesendet: Sonntag, 7. Mai 2023 10:17
An: Cyril Arnould<mailto:[email-masked]>
Cc: [email-masked]<mailto:[email-masked]>; Reto Zimmermann<mailto:[email-masked]>; Eli Zaretskii<mailto:[email-masked]>
Betreff: Re: bug#63251: 28.2; vhdl-mode contribution

The vhdl-mode maintainers need to look at your patch more closely; I just have some minor remarks.

7 maj 2023 kl. 00.11 skrev Cyril Arnould <[email-masked]>:

> - I've added TYPE to the vhdl-compiler definition with the
>   appropriate choices for Info/Warning/Error and the dotted
>   pair. I'm not sure if sexp was the correct choice for the
>   dotted pair, is there a better alternative?

There is (cons ...) which would be more precise, see the manual.

The new doc string says that a TYPE of 2 is allowed but the type spec doesn't allow it.
Either allow both 2 and nil or change the docs to only mention one of them.

> - I added another entry to the backwards compatibility code, all
>   it took was a slight modification of the entry before
>   that.

That's fine, but I'd be a bit more careful with the destructive in-place changes and quoted list constants. (Think of what happens if later code performs an in-place change of that nil you added.)
This isn't performance-critical-code, we can afford consing here.