Re: [patch] minor patch for register.el

[Stefan Monnier <monnier@iro.umontreal.ca>]

> 1. Adds functions `register-insertable-p', `register-jumpable-p', and
> `register-printable-p'.

Why are these needed?  The text description of the change sounds good,
but the corresponding code ends up duplicating a lot of `cond' code with
no obvious direct benefit.

The rest of the patch looks good (see further comments below).  Tho,
please don't turn the "(cond\n..." into "(cond ...)" which ends up
reindenting all the code gratuitously.

> @@ -148,12 +194,12 @@
>  delete any existing frames that the frame configuration doesn't mention.
>  \(Otherwise, these frames are iconified.)"
>    (interactive "cJump to register: \nP")
> +  (unless (get-register register)
> +    (error "No such register: `%s'" (single-key-description register)))
> +  (unless (register-jumpable-p register (called-interactively-p 'interactive))
> +    (error "Cannot jump to register `%s'" (single-key-description register)))
>    (let ((val (get-register register)))
> -    (cond
> -     ((registerv-p val)
> -      (cl-assert (registerv-jump-func val) nil
> -              "Don't know how to jump to register %s"
> -              (single-key-description register))
> +    (cond ((registerv-p val)
>        (funcall (registerv-jump-func val) (registerv-data val)))
>       ((and (consp val) (frame-configuration-p (car val)))
>        (set-frame-configuration (car val) (not delete))

Using cl-assert indeed sounds wrong.  It should probably be
a user-error instead.  But what is the benefit of moving the test?

> -	(message "Register %s is empty" (single-key-description register))
> +        (error "No such register: `%s'" (single-key-description register))

Make it a user-error.

> +  (if (null register-alist)
> +      (error "No registers to describe")

Same here.


        Stefan