emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: 7 logical-xor implementations in source tree


From: Basil L. Contovounesios
Subject: Re: 7 logical-xor implementations in source tree
Date: Thu, 01 Aug 2019 02:39:07 +0300
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)

Mattias Engdegård <address@hidden> writes:

> Attached is Basil's patch modified for a 2-arg `equiv' function, just as a
> reference point (and because code walks).

Thanks, LGTM except for some minor nits:

> +@defun equiv condition1 condition2
> +The @code{equiv} macro tests whether @var{condition1} and
                    ^^^^^
No longer a macro.

> +@var{condition2} are logically equivalent, i.e., either both
> +@code{nil} or both non-@code{nil}.
> +
> +If both arguments are non-@code{nil}, then the @code{equiv} call
> +returns the value of @var{condition2}.  If both arguments are
> +@code{nil}, @code{equiv} returns @code{t}.

What about when one is nil and the other isn't?

> +For example, the following expression tests whether either some state
> +is enabled (@var{enabled} is non-@code{nil}) and should be disabled
> +(@var{disable} is also non-@code{nil}), or the state is disabled
> +(@var{enabled} is @code{nil}) and should be enabled (@var{disable} is
> +also @code{nil}); if either of these conditions holds, the state
> +should subsequently be toggled:
> +
> +@example
> +(when (equiv enabled disable)
> +  ;; Toggle state
> +  @dots{})
> +@end example
> +@end defun

[I would love to see a better and less forced example than this, BTW.]

> +@defun xor condition1 condition2
> +This function returns the boolean exclusive-or of @var{condition1} and
> +@var{condition2}.  That is, @code{xor} returns @code{nil} if either
> +both arguments are @code{nil}, or both are non-@code{nil}.  Otherwise,
> +it returns the value of that argument which is non-@code{nil}.
> +
> +In other words, @code{xor} and @code{equiv} are the logical inverses
> +of each other.

Should each of these two definitions refer to the other, e.g. as "Note
that this is the logical inverse of..."?

> --- a/lisp/jsonrpc.el
> +++ b/lisp/jsonrpc.el
> @@ -43,9 +43,8 @@
>  (require 'warnings)
>  (require 'pcase)
>  (require 'ert) ; to escape a `condition-case-unless-debug'
> -(require 'array) ; xor
>  
> -
> +
>  ;;; Public API
>  ;;;

Is the whitespace change preferred?

> +(defsubst equiv (cond1 cond2)
> +  "Return non-nil if COND1 and COND2 are logically equivalent.
> +That is, they are either both nil, or both non-nil.
> +If neither argument is nil, returns COND2."

I think "return COND2" reads better in this context.

Thanks,

-- 
Basil



reply via email to

[Prev in Thread] Current Thread [Next in Thread]