[Top][All Lists]

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

Re: [patch] minor patch for register.el

From: Stefan Monnier
Subject: Re: [patch] minor patch for register.el
Date: Wed, 20 Feb 2013 12:05:04 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux)

> 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.


reply via email to

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