bug#6253: Please resize mini-buffer for queries even if resize-mini-windows is nil

bug#6253: Please resize mini-buffer for queries even if resize-mini-windows is nil

[Lennart Borgman]

Otherwise you just see the last line of a multiple line query prompt.

bug#6253: Please resize mini-buffer for queries even if resize-mini-windows is nil

[Stefan Monnier]

> Otherwise you just see the last line of a multiple line query prompt.

Hmm... IIUC when the user sets resize-mini-windows to nil, that's
because she doesn't want this resizing.
And multiline prompts are a problem (in cases where resizing is simply
impossible, for example) in any case.  So I think the right thing to do
is to report the "multiline prompts" as bugs.


        Stefan

bug#6253: Please resize mini-buffer for queries even if resize-mini-windows is nil

[Lennart Borgman]

On Sun, May 23, 2010 at 3:43 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> Otherwise you just see the last line of a multiple line query prompt.
>
> Hmm... IIUC when the user sets resize-mini-windows to nil, that's
> because she doesn't want this resizing.
> And multiline prompts are a problem (in cases where resizing is simply
> impossible, for example) in any case.  So I think the right thing to do
> is to report the "multiline prompts" as bugs.


I think what the user normally want is to avoid messages that resizes
the miniwindow. I guess most users want to see the whole multiline
prompt.

Why not distinguish between those cases? Maybe a new value for
resize-mini-windows.

With regards to multiline prompts please remember that a "short"
prompt may wrap because it includes something that is long. That could
for example be the case when the prompt includes a file path. So
beeing able to always resize the mini window is useful whatever we do.

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

[Stefan Monnier]

> Why not distinguish between those cases? Maybe a new value for
> resize-mini-windows.

Ah, you mean a setting that allows resizing minibuffers but not the
echo area.  Yes, that seems reasonable (tho it may bump into some
surprising cases where the user may think he's interacting with
a minibuffer whereas she's really looking at the echo area).

I'd first want to see a patch for it,


        Stefan

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

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

On Sun, May 23, 2010 at 4:10 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> Why not distinguish between those cases? Maybe a new value for
>> resize-mini-windows.
>
> Ah, you mean a setting that allows resizing minibuffers but not the
> echo area.  Yes, that seems reasonable (tho it may bump into some
> surprising cases where the user may think he's interacting with
> a minibuffer whereas she's really looking at the echo area).
>
> I'd first want to see a patch for it,


Thanks. I looked for read_filtered_event too in the sources.

I have attached a patch.

[-- Attachment #2: resize-mini-windows-1.diff --]
[-- Type: text/x-patch, Size: 6488 bytes --]



=== modified file 'src/fns.c'
--- 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);
 
@@ -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;
 	args[0] = Fpropertize (3, pargs);
 	args[1] = xprompt;
+        Vresize_mini_windows = Qt;
 	Fmessage (2, args);
       }
 
@@ -2637,6 +2665,8 @@
 
       temporarily_switch_to_single_kboard (SELECTED_FRAME ());
       obj = read_filtered_event (1, 0, 0, 0, Qnil);
+      message (0); /* Resize. */
+      Vresize_mini_windows = old_Vresize_mini_windows;
       cursor_in_echo_area = 0;
       /* If we need to quit, quit with cursor_in_echo_area = 0.  */
       QUIT;



=== modified file 'src/minibuf.c'
--- trunk/src/minibuf.c	2010-01-13 08:35:10 +0000
+++ patched/src/minibuf.c	2010-05-23 21:53:22 +0000
@@ -558,7 +558,7 @@
 
   /* If the minibuffer is on an iconified or invisible frame,
      make it visible now.  */
-  Fmake_frame_visible (mini_frame);
+  Fmake_frame_visible (mini_frame, Qnil);
 
   if (minibuffer_auto_raise)
     Fraise_frame (mini_frame);
@@ -576,13 +576,14 @@
 	     Fcons (minibuf_window,
 		    minibuf_save_list));
   minibuf_save_list
-    = Fcons (minibuf_prompt,
+    = Fcons (Vresize_mini_windows,
+         Fcons (minibuf_prompt,
 	     Fcons (make_number (minibuf_prompt_width),
 		    Fcons (Vhelp_form,
 			   Fcons (Vcurrent_prefix_arg,
 				  Fcons (Vminibuffer_history_position,
 					 Fcons (Vminibuffer_history_variable,
-						minibuf_save_list))))));
+						minibuf_save_list)))))));
 
   record_unwind_protect (read_minibuf_unwind, Qnil);
   minibuf_level++;
@@ -597,6 +598,7 @@
 
   minibuf_prompt_width = 0;
   minibuf_prompt = Fcopy_sequence (prompt);
+  Vresize_mini_windows = Qt;
   Vminibuffer_history_position = histpos;
   Vminibuffer_history_variable = histvar;
   Vhelp_form = Vminibuffer_help_form;
@@ -903,6 +905,7 @@
 {
   Lisp_Object old_deactivate_mark;
   Lisp_Object window;
+  Lisp_Object temp_Vresize_mini_windows;
 
   /* If this was a recursive minibuffer,
      tie the minibuffer window back to the outer level minibuffer buffer.  */
@@ -914,6 +917,14 @@
   Fset_buffer (XWINDOW (window)->buffer);
 
   /* Restore prompt, etc, from outer minibuffer level.  */
+  temp_Vresize_mini_windows = Fcar (minibuf_save_list);
+  minibuf_save_list = Fcdr (minibuf_save_list);
+  message (0); /* Resize. */
+  /* Check if changed in minibuffer, otherwise reset: */
+  if (Feq (Qt, Vresize_mini_windows))
+    {
+      Vresize_mini_windows = temp_Vresize_mini_windows;
+    }
   minibuf_prompt = Fcar (minibuf_save_list);
   minibuf_save_list = Fcdr (minibuf_save_list);
   minibuf_prompt_width = XFASTINT (Fcar (minibuf_save_list));



=== modified file 'src/lread.c'
--- trunk/src/lread.c	2010-04-20 01:50:52 +0000
+++ patched/src/lread.c	2010-05-23 20:13:31 +0000
@@ -800,11 +800,17 @@
      Lisp_Object prompt, inherit_input_method, seconds;
 {
   Lisp_Object val;
+  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, 1, ! NILP (inherit_input_method), seconds);
 
+  if (! NILP (prompt))
+      message (0); /* Resize. */
+  Vresize_mini_windows = old_Vresize_mini_windows;
+
   return (NILP (val) ? Qnil
 	  : make_number (char_resolve_modifier_mask (XINT (val))));
 }
@@ -822,9 +828,19 @@
      (prompt, inherit_input_method, seconds)
      Lisp_Object prompt, inherit_input_method, seconds;
 {
+  Lisp_Object val;
+  Lisp_Object old_Vresize_mini_windows = Vresize_mini_windows;
+  Vresize_mini_windows = Qt;
+
   if (! NILP (prompt))
     message_with_string ("%s", prompt, 0);
-  return read_filtered_event (0, 0, 0, ! NILP (inherit_input_method), seconds);
+  val = read_filtered_event (0, 0, 0, ! NILP (inherit_input_method), seconds);
+
+  if (! NILP (prompt))
+      message (0); /* Resize. */
+  Vresize_mini_windows = old_Vresize_mini_windows;
+
+  return val;
 }
 
 DEFUN ("read-char-exclusive", Fread_char_exclusive, Sread_char_exclusive, 0, 3, 0,
@@ -845,12 +861,18 @@
      Lisp_Object prompt, inherit_input_method, seconds;
 {
   Lisp_Object val;
+  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;
+
   return (NILP (val) ? Qnil
 	  : make_number (char_resolve_modifier_mask (XINT (val))));
 }







=== modified file 'src/lisp.h'
--- 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;

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

[Lars Magne Ingebrigtsen]

Lennart Borgman <lennart.borgman@gmail.com> writes:

>> Ah, you mean a setting that allows resizing minibuffers but not the
>> echo area.  Yes, that seems reasonable (tho it may bump into some
>> surprising cases where the user may think he's interacting with
>> a minibuffer whereas she's really looking at the echo area).

Yeah, sounds like a good idea.

> I have attached a patch.

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?

We also need a patch for the doc string and the manual, though.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

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

[Stefan Monnier]

> 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

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

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

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

That was nine years ago, so I doubt we're going to see much progress on
this.  I'm closing the bug report, but please reopen if we can proceed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no