Subject: Re: bug#6253: Please resize mini-buffer for queries even if

From: Stefan Monnier <[email-masked]>
Date: Wed, 11 Apr 2012 08:56:40 -0400

> I've read the patch, and it looks good to me.  Do we want to discuss
> this any further, or should I just apply the patch?

The patch I have (see below) doesn't look applicable as-is.
See comments below.

> --- trunk/src/fns.c	2010-04-25 22:16:58 +0000
> +++ patched/src/fns.c	2010-05-23 22:13:34 +0000
> @@ -2577,6 +2577,9 @@
>    Lisp_Object args[2];
>    struct gcpro gcpro1, gcpro2;
>    int count = SPECPDL_INDEX ();
> +  Lisp_Object old_Vresize_mini_windows = Vresize_mini_windows;
> +
> +  specbind (Qcursor_in_echo_area, Qt);
>  
>    specbind (Qcursor_in_echo_area, Qt);

I don't think we need to specbind the same variable twice ;-)

> @@ -2616,13 +2619,38 @@
 
>        {
>  	Lisp_Object pargs[3];
> -
> -	/* Colorize prompt according to `minibuffer-prompt' face.  */
> +        Lisp_Object face;
> +        Lisp_Object props;
> +        Lisp_Object use_face;
> +        Lisp_Object elt;
> +
> +        face = intern("face");
> +        props = Fsymbol_value (intern ("minibuffer-prompt-properties"));
> +        use_face = Qnil;
> +
> +        /* Colorize prompt according to `minibuffer-prompt-properties'.  */
> +        use_face = intern("default");
> +        while (1)
> +          {
> +            if (!CONSP (props) || EQ (XCAR (props), face))
> +              break;
> +
> +            props = XCDR (props);
> +          }
> +        if (CONSP (props))
> +          {
> +            props = XCDR (props);
> +            if (CONSP (props))
> +              {
> +                use_face = XCAR (props);
> +              }
> +          }
>  	pargs[0] = build_string ("%s(y or n) ");
>  	pargs[1] = intern ("face");
> -	pargs[2] = intern ("minibuffer-prompt");
> +        pargs[2] = use_face;

This seems unrelated.  Maybe it's a good change, but it should be
submitted separately, and motivated.

> -  Fmake_frame_visible (mini_frame);
> +  Fmake_frame_visible (mini_frame, Qnil);

That also seems unrelated.

> +  Vresize_mini_windows = Qt;

The main problem with the patch is that this new behavior cannot
be controlled.

> +  Lisp_Object old_Vresize_mini_windows = Vresize_mini_windows;
> +  Vresize_mini_windows = Qt;
> 
>    if (! NILP (prompt))
>      message_with_string ("%s", prompt, 0);
> 
>    val = read_filtered_event (1, 1, 0, ! NILP (inherit_input_method), seconds);
> 
> +  if (! NILP (prompt))
> +      message (0); /* Resize. */
> +  Vresize_mini_windows = old_Vresize_mini_windows;

This technique you've used at various places is unreliable.  You want to
use specbind+unbind instead.

> --- trunk/src/lisp.h	2010-05-15 21:19:05 +0000
> +++ patched/src/lisp.h	2010-05-23 15:47:47 +0000
> @@ -2625,6 +2625,7 @@
>  extern void safe_bcopy P_ ((const char *, char *, int));
> 
>  /* Defined in xdisp.c */
> +extern Lisp_Object Vresize_mini_windows;
>  extern Lisp_Object Qinhibit_point_motion_hooks;
>  extern Lisp_Object Qinhibit_redisplay, Qdisplay;
>  extern Lisp_Object Qinhibit_eval_during_redisplay;

This is not needed.

        Stefan