Re: Stop frames stealing eachothers' minibuffers!

[Stefan Monnier]

Hi Alan,

> I would be grateful indeed if either of you (or indeed, anybody else),
> would apply the patch and try it out, or indeed comment on it.  Thanks!

I'm not very familiar with this part of Emacs's code, but here are some
things I noticed:

> @@ -4416,7 +4416,8 @@ record-window-buffer
>         window (assq-delete-all buffer (window-prev-buffers window))))
>  
>      ;; Don't record insignificant buffers.
> -    (unless (eq (aref (buffer-name buffer) 0) ?\s)
> +    (when (or (not (eq (aref (buffer-name buffer) 0) ?\s))
> +              (string-match "^ \\*Minibuf" (buffer-name buffer)))

I think you want to use `minibufferp` here (and if not, then please add
a comment explaining why).

> @@ -4348,7 +4348,7 @@ extern void clear_regexp_cache (void);
>  
>  extern Lisp_Object Vminibuffer_list;
>  extern Lisp_Object last_minibuf_string;
> -extern void move_minibuffer_onto_frame (void);
> +extern void move_minibuffers_onto_frame (struct frame *, int);

Please use the `bool` type for booleans.

> +/* Get the next live buffer entry from a w->prev_buffer, setting the 
> pertinent
> +  variables of `zip_minibuffer_stacks'.  The parameter P is either "d" for
> +  the destination structures, or "s" for the source structures.  When the
> +  ->prev_buffers list is exhausted, set di/si to -1.  */
> +#define NEXT_BUFFER_ENTRY(p)                                         \
> +  do                                                                 \
> +    {                                                                        
> \
> +      if (NILP (p##_bufs))                                           \
> +        {                                                            \
> +       p##b = Qnil;                                                  \
> +       p##_ent = Qnil;                                               \
> +     }                                                               \
> +      else                                                           \
> +     {                                                               \
> +       p##_ent = Fcar (p##_bufs);                                    \
> +       p##b = Fcar (p##_ent);                                        \
> +       p##_bufs = Fcdr (p##_bufs);                                   \
> +     }                                                               \
> +      if (!NILP (p##b))                                                      
> \
> +     p##i = this_minibuffer_depth (p##b);                            \
> +      else                                                           \
> +     p##i = -1;                                                      \
> +    } while (p##i == 0)

Any chance we could make that into a function taking an argument `p`
that's a reference to a

   struct { Lisp_Object bufs; Lisp_Object b; Lisp_Object ent; int depth; }

?  I know we already use such longish macros in various places, but I'd
rather we use fewer of them rather than adding more of them, because
they're a pain to debug (and I find they make the code harder to read;
e.g. here we see a call to `NEXT_BUFFER_ENTRY` we can't know which
variables will be accessed/modified without looking at the definition of
the macro).

> +/* Move the ordered "stack" of minibuffers from SOURCE_WINDOW to
> +   DEST_WINDOW, interleaving those minibuffers with any in DEST_WINDOW
> +   to produce an ordered combination.  The ordering is by minibuffer
> +   depth.  A stack of minibuffers consists of the minibuffer currently
> +   in DEST/SOURCE_WINDOW together with any recorded in the
> +   ->prev_buffers field of the struct window.  */

This is actually the "merge" part of a merge sort, so we could use
`sort_list` or even just the `merge` function used by `sort_list`.

[ Tho maybe it is simpler and more robust (in case ELisp code can access
  (and hence modify) those lists) to leave those lists not sorted, and
  instead look for the deepest buffer in `minibuffer_unwind` rather than
  just picking the next.  ]

> +/* If `minibuffer_follows_selected_frame' is t, or we're about to
> +   delete a frame which potentially "contains" minibuffers, move them
> +   from the old frame to the selected frame.  This function is
> +   intended to be called from `do_switch_frame' in frame.c.  OF is the
> +   old frame, FOR_DELETION is self explanatory.  */

Actually, I suspect that the "self explanatory" part will not be true
when my daughter ends up having to fix that code 10 years from now.
[ Tho, it currently looks quite unlikely :-(  ]


        Stefan