Re: [elpa] externals/transient 667ce2b287 18/23: Use transient-default-value in transient-init-value(suffix)

Re: [elpa] externals/transient 667ce2b287 18/23: Use transient-default-value in transient-init-value(suffix)

[Stefan Monnier]

Hi Jonas,

> --- a/lisp/transient.el
> +++ b/lisp/transient.el
> @@ -3331,9 +3331,13 @@ Use `transient-default-value' to determine the default value."
>                (cdr saved)
>              (transient-default-value obj)))))
>  
> -(cl-defmethod transient-init-value ((_   transient-suffix))
> -  "Non-infix suffixes usually don't have a value, so this is a noop."
> -  nil)
> +(cl-defmethod transient-init-value ((obj transient-suffix))
> +  "Non-infix suffixes usually don't have a value.
> +Call `transient-default-value' but because that is a noop for
> +`transient-suffix', this function is effectively also a noop."
> +  (let ((value (transient-default-value obj)))
> +    (unless (eq value eieio--unbound)
> +      (oset obj value value))))
>  
>  (cl-defmethod transient-init-value ((obj transient-argument))
>    "Extract OBJ's value from the value of the prefix object."
> @@ -3379,6 +3383,11 @@ that.  If the slot is unbound, return nil."
>          default)
>      nil))
>  
> +(cl-defmethod transient-default-value ((_   transient-suffix))
> +  "Return `eieio--unbound' to indicate that there is no default value.
> +Doing so causes `transient-init-value' to skip setting the `value' slot."
> +  eieio--unbound)
> +
>  ;;;; Read
>  
>  (cl-defgeneric transient-infix-read (obj)

Why use `eieio--unbound` (i.e. a variable internal to EIEIO) rather than
your own magic placeholder?
IOW, is there a good reason to break the abstraction here?


        Stefan

Re: [elpa] externals/transient 667ce2b287 18/23: Use transient-default-value in transient-init-value(suffix)

[Jonas Bernoulli]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Hi Jonas,
>
>> --- a/lisp/transient.el
>> +++ b/lisp/transient.el
>> @@ -3331,9 +3331,13 @@ Use `transient-default-value' to determine the default value."
>>                (cdr saved)
>>              (transient-default-value obj)))))
>>  
>> -(cl-defmethod transient-init-value ((_   transient-suffix))
>> -  "Non-infix suffixes usually don't have a value, so this is a noop."
>> -  nil)
>> +(cl-defmethod transient-init-value ((obj transient-suffix))
>> +  "Non-infix suffixes usually don't have a value.
>> +Call `transient-default-value' but because that is a noop for
>> +`transient-suffix', this function is effectively also a noop."
>> +  (let ((value (transient-default-value obj)))
>> +    (unless (eq value eieio--unbound)
>> +      (oset obj value value))))
>>  
>>  (cl-defmethod transient-init-value ((obj transient-argument))
>>    "Extract OBJ's value from the value of the prefix object."
>> @@ -3379,6 +3383,11 @@ that.  If the slot is unbound, return nil."
>>          default)
>>      nil))
>>  
>> +(cl-defmethod transient-default-value ((_   transient-suffix))
>> +  "Return `eieio--unbound' to indicate that there is no default value.
>> +Doing so causes `transient-init-value' to skip setting the `value' slot."
>> +  eieio--unbound)
>> +
>>  ;;;; Read
>>  
>>  (cl-defgeneric transient-infix-read (obj)
>
> Why use `eieio--unbound` (i.e. a variable internal to EIEIO) rather than
> your own magic placeholder?

I first used `eieio--unbound' "manually" (in another package), when it
was still named `eieio-unbound'.  When you made that breaking change,
I quickly added the necessary backward compatibility alias to that
package.  I have (in a later) commit done the same here.  (I briefly
forgot that this is not the package in which I had already been using
this symbol.  This may have contributed to me using this symbol here;
I though I was already using it.)

The reason I am using Eieio's "this is not bound", is that I am doing an
Eieio thing here.  It's not a "not bound" slot but the "not bound"
return value of a generic function, which isn't the same thing, but
pretty damn close.

Sure I can invent a new symbol just for this one case, but it would be
(IMO unnecessary) noise and the only reasons I would be doing it is to
make you happy and to avoid having a conversation about it.

> IOW, is there a good reason to break the abstraction here?

I am not sure what "the abstraction" and "here" refer to exactly.