behavior of *, ** and *** in ielm: patch.

[Luc Teirlinck <teirllm@dms.auburn.edu>]

Below are two ielm runs illustrating two unrelated bugs in ielm.

One bug is that killing the current buffer in ielm produces a
confusing bogus error message (see first run below).  The second run
shows the correct behavior after the patch.  Actually, the patch does
not explicitly do anything to correct the bug, because taking care of
the next bug automatically eliminated this one too.

The second bug is that the present behavior of *, ** and *** is
inconsistent and, in more complex situations, essentially
unpredictable.

Everything is fine as long as the ielm buffer stays the current
buffer.  However, switching between different current buffers causes
problems.  Indeed, if one evaluates an expression with the ielm buffer
as the original working buffer, the the values of *, ** and *** are
not accessible in other buffers.  (See the first run below.)  This is
very inconvenient and counterintuitive.  It is also inconsistent with
the behavior when the original working buffer is a non-ielm buffer,
Indeed, then the values of the * variables are accessible in all
buffers, except other ielm buffers, which shadow them, except when
they themselves were current at the start of evaluation. (The exact
"rules" are complicated.)  If one puts a current buffer in ielm mode,
which actually sometimes is useful in practice, then all bets are off.
The values of the * variables in the process buffer and the new second
ielm buffer can get scrambled.  Killing a current buffer further
complicates the "rules" and triggers the first bug, described above.

Next, two ielm runs, illustrating the problems in the current
behavior.  The first run is before applying the patch, the second
after.

===File ~/oldielmlog========================================
*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (setq buf1 (get-buffer-create "buf1"))
#<buffer buf1>
ELISP> (set-buffer buf1)
#<buffer buf1>
ELISP> (kill-buffer buf1)
*** Eval error ***  error: "Selecting deleted buffer"
ELISP> (current-buffer)
#<buffer *scratch*>
ELISP> (list *** ** *)
(#<killed buffer> #<killed buffer> #<buffer *scratch*>)

ELISP> (set-buffer "*ielm*")
#<buffer *ielm*>
ELISP> (progn (set-buffer "*scratch*") (list *** ** *))
(nil nil nil)

ELISP> 
============================================================


===File ~/newielmlog========================================
*** Welcome to IELM ***  Type (describe-mode) for help.
ELISP> (setq buf1 (get-buffer-create "buf1"))
#<buffer buf1>
ELISP> (set-buffer buf1)
#<buffer buf1>
ELISP> (kill-buffer buf1)
t
ELISP> (kill-buffer buf1)
nil
ELISP> (current-buffer)
#<buffer *scratch*>
ELISP> (list *** ** *)
(t nil #<buffer *scratch*>)

ELISP> (set-buffer "*ielm*")
#<buffer *ielm*>
ELISP> (progn (set-buffer "*scratch*") (list *** ** *))
(#<buffer *scratch*>
	  (t nil #<buffer *scratch*>)
	  #<buffer *ielm*>)

ELISP> 
============================================================

The rules (valid during ielm evaluation) for the * variables
implemented by the patch below are the following:

1.  The values of *, ** and *** are valid in all buffers, except other
    ielm buffers which shadow them with their own local bindings.
    That is, during ielm evaluation, these normally ielm-local
    variables behave like global variables.  As such, they are
    shadowed by buffer-local bindings, which normally only exist in
    other ielm buffers.

2.  If the current buffer is an ielm buffer distinct from the process
    buffer, then the shadowed process buffer values can be accessed
    using the variables *1, *2 and *3, which are normally just aliases
    for *, ** and ***, except in this particular situation.

These rules are implemented consistently, even in the most complex
situations where one makes two different ielm buffers current in one
another, switches between the two, puts current buffers in ielm mode,
kills current buffers, whatever.  

The variables *1, *2 and *3 are not really newly introduced variables.
Instead they are just new names for the old technical local variables
*save, **save and ***save.  They only get bound to their intended
value during ielm evaluation.  The rationale for the added emphasis on
them is that the situation in which they become useful is less
infrequent than might seem at first.  If one wants to operate on an
ielm buffer using ielm, then one quite often can not use the ielm
buffer itself, because evaluating an expression produces changes in
the ielm buffer due to the act of evaluating, rather than due to the
actual expression being evaluated.  Hence, the expressions have to be
evaluated from another ielm buffer.  There are a variety of other
reasons why having more than one ielm buffer sometimes is useful.  We
need to use these variables anyway for technical reasons.  All we need
to do is give them convenient names and tell the user about them.

In the patch below, telling the user about them means a pair of
additional lines in the mode documentation string and defvars, with
documentation strings, for the three variables.

Technically, the way the new behavior is implemented is to make a
temporary buffer current at entry to the inner of the two lets in the
double let construct, kill it before evaluating the ielm form and then
make a second temporary buffer current at exit to the let, killing it
after exit.  This may sound contorted, but it is the only way to
produce completely predictable consistent behavior.  We need to make
sure that the let binds global bindings and that neither the buffer
current at entry, nor the buffer current at exit, are alive (and hence
could potentially become current) during evaluation of ielm-form.
Indeed, Gerd's recent bug fix for let still does not prevent
make-local-variable from scrambling bindings if called within a let
with a current buffer that was current on entry or will be current on
exit.  We also do not want mysterious temporary buffers to show up in
(buffer-list) or otherwise cause trouble.

The patch below also contains a very minor additional change in the
behavior of *, **, and ***.  In the function inferior-emacs-lisp-mode,
the patch interchanges the order of the original:

(setq * nil)
(make-local-variable '*)

to:

(make-local-variable '*)
(setq * nil)

and similar for ** and ***.

The rationale is that, although it is unlikely that the user would
make global bindings for these variables, overruling such bindings,
outside of ielm evaluation, is completely gratuitous.  Ielm never uses
or gets confused by such global bindings anyway, although, of course,
they would not be accessible during ielm evaluation.  Similarly, user
global bindings for *1, *2 and *3 will not be overridden, except
during ielm evaluation, and will not cause problems.

I have previously signed papers in connection with proposed changes to
mailabbrev.el.  Gerd told me that these papers would cover other work
as well.  So, I guess that if you agree with the patch below we do not
need to worry about papers.  (Unless I misunderstood Gerd.)

Next a Change log and the diff.
     
2002-07-20  Luc Teirlinck  <teirllm@mail.auburn.edu>

	* ielm.el: (*1, *2, *3): New variables.
	(ielm-eval-input): Make temporary buffers current on entry and 
        exit to the let bindings for *, ** and ***.
	(inferior-emacs-lisp-mode): Mention *1, *2 and *3 in the docstring.
        Do not overrule global bindings for *, ** and ***.


In the next diff, oldielm.el is the Emacs21.2.90 version of ielm.el.

===File ~/ielmdiff==========================================
cd /usr/local/share/emacs/21.2.90/lisp/
diff -c /usr/local/share/emacs/21.2.90/lisp/oldielm.el /usr/local/share/emacs/21.2.90/lisp/ielm.el
*** /usr/local/share/emacs/21.2.90/lisp/oldielm.el	Tue Oct 10 12:27:38 2000
--- /usr/local/share/emacs/21.2.90/lisp/ielm.el	Thu Jul 18 16:25:16 2002
***************
*** 103,108 ****
--- 103,129 ----
  (defvar *** nil
    "Third-most-recent value evaluated in IELM.")
  
+ (defvar *1 nil
+   "During IELM evaluation, most recent value evaluated in IELM.
+ Normally identical to `*'.  However, if the working buffer is an IELM
+ buffer, distinct from the process buffer, then `*' gives the value in
+ the working buffer, *1 the value in the process buffer.  
+ The intended value is only accessible during IELM evaluation.")
+ 
+ (defvar *2 nil
+   "During IELM evaluation, second-most-recent value evaluated in IELM.
+ Normally identical to `**'.  However, if the working buffer is an IELM
+ buffer, distinct from the process buffer, then `**' gives the value in
+ the working buffer, *2 the value in the process buffer.
+ The intended value is only accessible during IELM evaluation.")
+ 
+ (defvar *3 nil
+   "During IELM evaluation, third-most-recent value evaluated in IELM.
+ Normally identical to `***'.  However, if the working buffer is an IELM
+ buffer, distinct from the process buffer, then `***' gives the value in
+ the working buffer, *3 the value in the process buffer.
+ The intended value is only accessible during IELM evaluation.")
+ 
  ;;; System variables
  
  (defvar ielm-working-buffer nil
***************
*** 308,336 ****
  		      ielm-error-type "IELM Error"
  		      ielm-wbuf (current-buffer))
  	      (if (ielm-is-whitespace (substring ielm-string ielm-pos))
! 		  ;; need this awful let convolution to work around
! 		  ;; an Emacs bug involving local vbls and let binding
! 		  (let ((*save *)
! 			(**save **)
! 			(***save ***))
  		    (save-excursion
! 		      (set-buffer ielm-working-buffer)
! 		      (condition-case err
! 			  (let ((* *save)
! 				(** **save)
! 				(*** ***save)
! 				(ielm-obuf (current-buffer)))
! 			    (setq ielm-result (eval ielm-form))
! 			    (setq ielm-wbuf (current-buffer))
! 			    ;; The eval may have changed current-buffer;
! 			    ;; need to set it back here to avoid a bug
! 			    ;; in let.  Don't want to use save-excursion
! 			    ;; because we want to allow changes in point.
! 			    (set-buffer ielm-obuf))
! 			(error (setq ielm-result (ielm-format-error err))
! 			       (setq ielm-error-type "Eval error"))
! 			(quit (setq ielm-result "Quit during evaluation")
! 			      (setq ielm-error-type "Eval error")))))
  		(setq ielm-error-type "IELM error")
  		(setq ielm-result "More than one sexp in input"))))
  
--- 329,378 ----
  		      ielm-error-type "IELM Error"
  		      ielm-wbuf (current-buffer))
  	      (if (ielm-is-whitespace (substring ielm-string ielm-pos))
! 		  ;; To correctly handle the ielm-local variables *,
! 		  ;; ** and ***, we need a temporary buffer to be
! 		  ;; current at entry to the inner of the next two let
! 		  ;; forms.  We need another temporary buffer to exit
! 		  ;; that same let.  To avoid problems, neither of
! 		  ;; these buffers should be alive during the
! 		  ;; evaluation of ielm-form.
! 		  (let ((*1 *)
! 			(*2 **)
! 			(*3 ***)
! 			ielm-temp-buffer)
  		    (save-excursion
! 		      (with-temp-buffer
! 			(condition-case err
! 			    (unwind-protect
! 				;; The next let form creates default
! 				;; bindings for *, ** and ***.  But
! 				;; these default bindings are
! 				;; identical to the ielm-local
! 				;; bindings.  Hence, during the
! 				;; evaluation of ielm-form, the
! 				;; ielm-local values are going to be
! 				;; used in all buffers except for
! 				;; other ielm buffers, which override
! 				;; them.  Normally, the variables *1,
! 				;; *2 and *3 also have default
! 				;; bindings, which are not overridden.
! 				(let ((* *1)
! 				      (** *2)
! 				      (*** *3))
! 				  (kill-buffer (current-buffer))
! 				  (set-buffer ielm-wbuf)
! 				  (setq ielm-result (eval ielm-form))
! 				  (setq ielm-wbuf (current-buffer))
! 				  (setq
! 				   ielm-temp-buffer
! 				   (generate-new-buffer " *ielm-temp*"))
! 				  (set-buffer ielm-temp-buffer))
! 			      (when ielm-temp-buffer
! 				(kill-buffer ielm-temp-buffer)))
! 			  (error (setq ielm-result (ielm-format-error err))
! 				 (setq ielm-error-type "Eval error"))
! 			  (quit (setq ielm-result "Quit during evaluation")
! 				(setq ielm-error-type "Eval error"))))))
  		(setq ielm-error-type "IELM error")
  		(setq ielm-result "More than one sexp in input"))))
  
***************
*** 395,404 ****
  * \\[comint-dynamic-complete] completes Lisp symbols (or filenames, within strings),
    or indents the line if there is nothing to complete.
  
- During evaluations, the values of the variables `*', `**', and `***'
- are the results of the previous, second previous and third previous
- evaluations respectively.
- 
  The current working buffer may be changed (with a call to
  `set-buffer', or with \\[ielm-change-working-buffer]), and its value
  is preserved between successive evaluations.  In this way, expressions
--- 437,442 ----
***************
*** 406,411 ****
--- 444,455 ----
  Display the name of the working buffer with \\[ielm-print-working-buffer],
  or the buffer itself with \\[ielm-display-working-buffer].
  
+ During evaluations, the values of the variables `*', `**', and `***'
+ are the results of the previous, second previous and third previous
+ evaluations respectively.  If the working buffer is another IELM
+ buffer, then the values in the working buffer are used.  The variables
+ `*1', `*2' and `*3', yield the process buffer values.
+ 
  Expressions evaluated by IELM are not subject to `debug-on-quit' or
  `debug-on-error'.
  
***************
*** 445,456 ****
    (setq fill-paragraph-function 'lisp-fill-paragraph)
  
    ;; Value holders
-   (setq * nil)
    (make-local-variable '*)
!   (setq ** nil)
    (make-local-variable '**)
!   (setq *** nil)
    (make-local-variable '***)
  
    ;; font-lock support
    (make-local-variable 'font-lock-defaults)
--- 489,500 ----
    (setq fill-paragraph-function 'lisp-fill-paragraph)
  
    ;; Value holders
    (make-local-variable '*)
!   (setq * nil)
    (make-local-variable '**)
!   (setq ** nil)
    (make-local-variable '***)
+   (setq *** nil)
  
    ;; font-lock support
    (make-local-variable 'font-lock-defaults)

Diff finished at Fri Jul 19 20:07:17
============================================================