Philip Kaluderic <philipk@posteo.net> writes:
> Zachary D'Anthony <zdm@disroot.org> writes:
>
> > Greetings,
>
> Hi,
>
> > I was looking over your emacs init file and noticed you never use the
> > local macro `:option' for setting custom variables, but rather use
> >
> > (setopt ...)
> >
> > Is there a reason for this?
>
> I wrote :option before there was any setopt.
>
> ...
>
> As for setopt, it provides primitive type checking for user options,
> which is useful most of the time.
>
> [0] https://www.emacswiki.org/emacs/SetupEl#h5o-20
>
> --
> Philip Kaludercic on peregrine
Hello,
Would you be open to adding the primitive type checking that `setopt`
does to `:option`? I added it in a local version, and it and `setopt`
have been helpful in finding errors and mistakes in my config. It is
convenient to have the type checking while keeping the convenience of
Setup's setters, such as `append*`.
I can provide a patch file, if you are interested.
Thank you.
Okamsn <okamsn@protonmail.com> writes:
> Philip Kaluderic <philipk@posteo.net> writes:> > Zachary D'Anthony <zdm@disroot.org> writes:> >> > > Greetings,> >> > Hi,> >> > > I was looking over your emacs init file and noticed you never use the> > > local macro `:option' for setting custom variables, but rather use> > >> > > (setopt ...)> > >> > > Is there a reason for this?> >> > I wrote :option before there was any setopt.> >> > ...> >> > As for setopt, it provides primitive type checking for user options,> > which is useful most of the time.> >> > [0] https://www.emacswiki.org/emacs/SetupEl#h5o-20> >> > --> > Philip Kaludercic on peregrine>> Hello,>> Would you be open to adding the primitive type checking that `setopt` > does to `:option`? I added it in a local version, and it and `setopt` > have been helpful in finding errors and mistakes in my config. It is > convenient to have the type checking while keeping the convenience of > Setup's setters, such as `append*`.>> I can provide a patch file, if you are interested.
Sure! But couldn't we just re-use setup on newer versions?
> Thank you.>>
--
Philip Kaludercic on siskin
Philip Kaludercic wrote:
>> Hello,>>>> Would you be open to adding the primitive type checking that `setopt`>> does to `:option`? I added it in a local version, and it and `setopt`>> have been helpful in finding errors and mistakes in my config. It is>> convenient to have the type checking while keeping the convenience of>> Setup's setters, such as `append*`.>>>> I can provide a patch file, if you are interested.> > Sure! But couldn't we just re-use setup on newer versions?> >> Thank you.>>>>> > --> Philip Kaludercic on siskin
Attached is a patch that uses `setopt` directly when it is defined.
Would you like any changes to it?
Okamsn <okamsn@protonmail.com> writes:
> Philip Kaludercic wrote:>>> Hello,>>>>>> Would you be open to adding the primitive type checking that `setopt`>>> does to `:option`? I added it in a local version, and it and `setopt`>>> have been helpful in finding errors and mistakes in my config. It is>>> convenient to have the type checking while keeping the convenience of>>> Setup's setters, such as `append*`.>>>>>> I can provide a patch file, if you are interested.>> >> Sure! But couldn't we just re-use setup on newer versions?>> >>> Thank you.>>>>>>>> >> -->> Philip Kaludercic on siskin>>> Attached is a patch that uses `setopt` directly when it is defined. > Would you like any changes to it?>> From 45c5ad4e4ad251256eb70f8f6b5a18285512a737 Mon Sep 17 00:00:00 2001> From: Earl Hyatt <okamsn@protonmail.com>> Date: Fri, 6 Sep 2024 19:50:38 -0400> Subject: [PATCH] Add primitive type checking to `:option`.>> The new `setopt` macro in Emacs 29 can perform a basic check to see whether> the value being used matches the variable's customization type. Use that logic> for the `:option` Setup macro if `setopt` is not defined, otherwise use `setopt`> directly.> ---> setup.el | 19 +++++++++++++++----> 1 file changed, 15 insertions(+), 4 deletions(-)>> diff --git a/setup.el b/setup.el> index cf75ad8..4a70aac 100644> --- a/setup.el> +++ b/setup.el> @@ -662,10 +662,21 @@ (setup-define :option> #'symbol-value)> ',name))> (lambda (name val)> - `(progn> - (custom-load-symbol ',name)> - (funcall (or (get ',name 'custom-set) #'set-default)> - ',name ,val))))> + (if (fboundp 'setopt)> + `(setopt ,name ,val)> + (macroexp-let2 nil val val> + `(progn> + (custom-load-symbol ',name)> + ;; Perform the same primitive type checking as `setopt', which was> + ;; added in Emacs 29. `setopt' puts the property> + ;; `custom-check-value' on the symbol naming the variable, which is> + ;; used by `custom-initialize-reset'.> + (when-let ((type (get ',name 'custom-type)))> + (unless (widget-apply (widget-convert type) :match ,val)> + (warn "`%s': value `%S' does not match type %s" ',name ,val type)))> + (put ',name 'custom-check-value (list ,val))> + (funcall (or (get ',name 'custom-set) #'set-default)> + ',name ,val))))))
While I get what you are doing, I am uncomfortable with the added
complexity this introduced to the reader, especially for a local-macro
I'd rather like to deprecate. Would you mind it if I took your macro
and just removed the type-checking pre-29?
> > :documentation "Set the option NAME to VAL.> NAME may be a symbol, or a cons-cell. If NAME is a cons-cell, it
--
Philip Kaludercic on siskin
Philip Kaludercic wrote:
>> From 45c5ad4e4ad251256eb70f8f6b5a18285512a737 Mon Sep 17 00:00:00 2001>> From: Earl Hyatt <okamsn@protonmail.com>>> Date: Fri, 6 Sep 2024 19:50:38 -0400>> Subject: [PATCH] Add primitive type checking to `:option`.>>>> The new `setopt` macro in Emacs 29 can perform a basic check to see whether>> the value being used matches the variable's customization type. Use that logic>> for the `:option` Setup macro if `setopt` is not defined, otherwise use `setopt`>> directly.>> --->> setup.el | 19 +++++++++++++++---->> 1 file changed, 15 insertions(+), 4 deletions(-)>>>> diff --git a/setup.el b/setup.el>> index cf75ad8..4a70aac 100644>> --- a/setup.el>> +++ b/setup.el>> @@ -662,10 +662,21 @@ (setup-define :option>> #'symbol-value)>> ',name))>> (lambda (name val)>> - `(progn>> - (custom-load-symbol ',name)>> - (funcall (or (get ',name 'custom-set) #'set-default)>> - ',name ,val))))>> + (if (fboundp 'setopt)>> + `(setopt ,name ,val)>> + (macroexp-let2 nil val val>> + `(progn>> + (custom-load-symbol ',name)>> + ;; Perform the same primitive type checking as `setopt', which was>> + ;; added in Emacs 29. `setopt' puts the property>> + ;; `custom-check-value' on the symbol naming the variable, which is>> + ;; used by `custom-initialize-reset'.>> + (when-let ((type (get ',name 'custom-type)))>> + (unless (widget-apply (widget-convert type) :match ,val)>> + (warn "`%s': value `%S' does not match type %s" ',name ,val type)))>> + (put ',name 'custom-check-value (list ,val))>> + (funcall (or (get ',name 'custom-set) #'set-default)>> + ',name ,val))))))> > While I get what you are doing, I am uncomfortable with the added> complexity this introduced to the reader, especially for a local-macro> I'd rather like to deprecate. Would you mind it if I took your macro> and just removed the type-checking pre-29?
I think that not having the type checking is a missed opportunity for
those that do no yet have access to `setopt`, but if you intend to
eventually remove the local macro anyway, then I think those differences
between Emacs versions will become unimportant.
Please do what you think is best, since you are the one who has to
maintain it in the end. I do not mind.
Okamsn <okamsn@protonmail.com> writes:
> Philip Kaludercic wrote:>>> From 45c5ad4e4ad251256eb70f8f6b5a18285512a737 Mon Sep 17 00:00:00 2001>>> From: Earl Hyatt <okamsn@protonmail.com>>>> Date: Fri, 6 Sep 2024 19:50:38 -0400>>> Subject: [PATCH] Add primitive type checking to `:option`.>>>>>> The new `setopt` macro in Emacs 29 can perform a basic check to see whether>>> the value being used matches the variable's customization type. Use that logic>>> for the `:option` Setup macro if `setopt` is not defined, otherwise use `setopt`>>> directly.>>> --->>> setup.el | 19 +++++++++++++++---->>> 1 file changed, 15 insertions(+), 4 deletions(-)>>>>>> diff --git a/setup.el b/setup.el>>> index cf75ad8..4a70aac 100644>>> --- a/setup.el>>> +++ b/setup.el>>> @@ -662,10 +662,21 @@ (setup-define :option>>> #'symbol-value)>>> ',name))>>> (lambda (name val)>>> - `(progn>>> - (custom-load-symbol ',name)>>> - (funcall (or (get ',name 'custom-set) #'set-default)>>> - ',name ,val))))>>> + (if (fboundp 'setopt)>>> + `(setopt ,name ,val)>>> + (macroexp-let2 nil val val>>> + `(progn>>> + (custom-load-symbol ',name)>>> + ;; Perform the same primitive type checking as `setopt', which was>>> + ;; added in Emacs 29. `setopt' puts the property>>> + ;; `custom-check-value' on the symbol naming the variable, which is>>> + ;; used by `custom-initialize-reset'.>>> + (when-let ((type (get ',name 'custom-type)))>>> + (unless (widget-apply (widget-convert type) :match ,val)>>> + (warn "`%s': value `%S' does not match type %s" ',name ,val type)))>>> + (put ',name 'custom-check-value (list ,val))>>> + (funcall (or (get ',name 'custom-set) #'set-default)>>> + ',name ,val))))))>> >> While I get what you are doing, I am uncomfortable with the added>> complexity this introduced to the reader, especially for a local-macro>> I'd rather like to deprecate. Would you mind it if I took your macro>> and just removed the type-checking pre-29?>> I think that not having the type checking is a missed opportunity for > those that do no yet have access to `setopt`, but if you intend to > eventually remove the local macro anyway, then I think those differences > between Emacs versions will become unimportant.
Off-remark: An idea I have been playing with, that I don't have the time
to start is a user-facing version of the "Compat" package that would
include useful configuration macros and other functions replicated for
older versions of Emacs. That is where I would prefer to add such
functionality.
> Please do what you think is best, since you are the one who has to > maintain it in the end. I do not mind.
I will think about it. Perhaps it would be enough to just add a comment
linking to this thread to clarify my concerns?
--
Philip Kaludercic on siskin