slime-macroexpand-again when buffer not a macroexpansion buffer

Reported by mon_key on 2011-05-04
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Slime
Undecided
Unassigned

Bug Description

`slime-macroexpand-again' is an interactive function. When current-buffer is not a macroexpansion-buffer evaluation of `slime-macroexpand-again' will clobber the contents of the macroexpansion-buffer.

mon_key (mon-key) wrote :

What this means is that when one evaluates M-x slime-macroexpand-again in a lisp file which is not a macroexpansion-buffer e.g. some-file-i-do-not-want-clobbered.lisp the contents of the file _are_ clobbered by slime-macroexpand-again and left in a more or less unrecoverable state! This happens easily when one uses values of `minibuffer-history' for M-x command completion esp. b/c `slime-macroexpand-*' is a prefix for at least seven different interactive macro-expansion related commands including:
 slime-macroexpand-1
 slime-macroexpand-1-inplace
 slime-macroexpand-again
 slime-macroexpand-all
 slime-macroexpand-all-inplace
 slime-macroexpand-undo
 slime-macroexpansion-minor-mode

The Interactive `slime-macroexpand-*' commands which might potentially clobber the contents of current-buffer should check that current-buffer is in fact a macro-expansion buffer before proceeding...

--
/s_P\

mon_key (mon-key) wrote :

Following seems to fix the problem:

(defun slime-macroexpand-again ()
  "Reperform the last macroexpansion."
  (interactive)
  (slime-eval-async slime-eval-macroexpand-expression
    (slime-rcurry #'slime-initialize-macroexpansion-buffer
                 ;; :WAS (current-buffer)
                  (slime-buffer-name :macroexpansion))))

The problem seems to be mostly around `slime-initialize-macroexpansion-buffer' which does this:
 (setq buffer-undo-list nil)

IOW the currying in the `slime-macroexpand-again' causes the buffer-undo-list of whatever (current-buffer) returns to loose its undo-list... passing `slime-rcurry' an expliclit buffer (e.g. "*slime-macroexpansion*") should prevent current-buffer from loosing its undo-list when it isn't a "*slime-macroexpansion* buffer.

--
/s_P\

Stas Boukarev (stassats) wrote :

Why do you run slime-macroexpand-again not in a slime-macroexpansion buffer?

On Sun, May 8, 2011 at 7:12 AM, Stas Boukarev <email address hidden> wrote:
> Why do you run slime-macroexpand-again not in a slime-macroexpansion
> buffer?
>

On Sun, May 8, 2011 at 7:12 AM, Stas Boukarev <email address hidden> wrote:
> Why do you run slime-macroexpand-again not in a slime-macroexpansion
> buffer?
>

As indicated earlier, an invocation of `slime-macroexpand-again' in
the wrong buffer is most often by accident:

,----
| This happens easily when one uses values of `minibuffer-history' for
| M-x command completion esp. b/c `slime-macroexpand-*' is a prefix
| for at least seven different interactive macro-expansion related
| commands including:
|
| slime-macroexpand-1
| slime-macroexpand-1-inplace
| slime-macroexpand-again
| slime-macroexpand-all
| slime-macroexpand-all-inplace
| slime-macroexpand-undo
| slime-macroexpansion-minor-mode
|
`----

Regardless, I don't see any reason why the proposed modification of
`slime-macroexpand-again' shouldn't slime-rcurry (slime-buffer-name
:macroexpansion) instead of (current-buffer) e.g.:

(defun slime-macroexpand-again ()
 "Reperform the last macroexpansion."
 (interactive)
 (slime-eval-async slime-eval-macroexpand-expression
   (slime-rcurry #'slime-initialize-macroexpansion-buffer
                ;; :WAS (current-buffer)
                 (slime-buffer-name :macroexpansion))))

Do you know of some reason why the above proposal would constitute a
breaking change?

--
/s_P\

Stas Boukarev (stassats) wrote :

Why would you run M-x slime-macroexpand-again manualy, it's bound to g only in the slime-macroexpansion mode. And I prefer not to fix what's not broken.

mon_key (mon-key) wrote :

> Why would you run M-x slime-macroexpand-again manualy, it's bound to g only in the slime-macroexpansion mode.

What part of "It can easily happen by accident with minibuffer-complete" do you not understand?

Just because it doesn't happen to Stas or fit your usage pattern doesn't mean it isn't a bug.

`slime-macroexpand-again' relies on functions which set buffer-undo-list nil _and_ erase-buffer. When this happens in a non "*slime-macroexpansion*" buffer the end result is I loose the contents of whatever changes have been made since the last autosave/save/commit.

The proposed fix for this problem provided above is easy, clean, and does not break the semantics of existing slime features.

However, in so much as you will not consider this a bug I suggest we move the discussion there -- maybe others can provide additional insight. As such, I've forwarded a patch with more extensive changes related to the problem to slime-devel.

--
/s_P\

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers