bug#7307: 24.0.50; Mode line had more than just dashes removed

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

This change:

2010-10-18  Julien Danjou  <julien@danjou.info>

	* bindings.el: Remove end dashes in default mode-line-format.

not only removed the dashes at the end of the mode line, but also the
help string in a tool tip together with the altered mouse pointer, which
serves as a visual cue that the mode line is draggable (with vertically
split windows).  I think these help indicators should be restored.
Unfortunately, there is no mode line construct that inserts spaces to
the end of the window like "%-" inserts dashed.  So I tried make-string
and it seems to DTRT.  The only problem is specifying the length of the
string: (window-width) as the maximum possible length fails here,
because the mode line is constructed before the window-system frame, and
has a value of just 10.  So I just picked 200 as a plausible maximum.
Maybe there's a better way, but this seems to work ok.

*** /home/steve/bzr/emacs/trunk/lisp/bindings.el	2010-10-24 13:56:16.000000000 +0200
--- /home/steve/bzr/emacs/quickfixes/lisp/bindings.el	2010-10-30 00:39:06.000000000 +0200
***************
*** 336,342 ****
  	 'mode-line-modes
  	 `(which-func-mode ("" which-func-format ,spaces))
  	 `(global-mode-string ("" global-mode-string ,spaces))
! 	 `(:eval (unless (display-graphic-p)
  		   ,(propertize "-%-" 'help-echo help-echo)))))
         (standard-mode-line-modes
  	(list
--- 336,343 ----
  	 'mode-line-modes
  	 `(which-func-mode ("" which-func-format ,spaces))
  	 `(global-mode-string ("" global-mode-string ,spaces))
! 	 `(:eval (if (display-graphic-p)
! 		     ,(propertize (make-string 200 32) 'help-echo help-echo)
  		   ,(propertize "-%-" 'help-echo help-echo)))))
         (standard-mode-line-modes
  	(list

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Sat, 30 Oct 2010 01:05:32 +0200
> Cc: 
> 
> The only problem is specifying the length of the
> string: (window-width) as the maximum possible length fails here,
> because the mode line is constructed before the window-system frame, and
> has a value of just 10.  So I just picked 200 as a plausible maximum.
> Maybe there's a better way, but this seems to work ok.

Would it help to use window-width as the maximum?

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

On Sat, 30 Oct 2010 08:51:14 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Date: Sat, 30 Oct 2010 01:05:32 +0200
>> Cc: 
>> 
>> The only problem is specifying the length of the
>> string: (window-width) as the maximum possible length fails here,
>> because the mode line is constructed before the window-system frame, and
>> has a value of just 10.  So I just picked 200 as a plausible maximum.
>> Maybe there's a better way, but this seems to work ok.
>
> Would it help to use window-width as the maximum?

?? See above.  Or did you mean something else?  (Though I see that what
I wrote is a bit garbled: it should say "(window-width) has a value of
just 10".)  Anyway, maybe it's less kludgy to introduce a mode line
%-construct for spaces.  Here's a patch for that, which just adapts the
code for "%-".  It seems to works (but so does my other lisp-only
patch).  (Interestingly, lots_of_dashes, and hence lots_of_spaces, has a
length of 140, but that's enough at least for my screen.)

=== modified file 'doc/lispref/modes.texi'
--- doc/lispref/modes.texi	2010-08-22 19:30:26 +0000
+++ doc/lispref/modes.texi	2010-10-30 13:01:23 +0000
@@ -2029,6 +2029,9 @@
 @item %-
 Dashes sufficient to fill the remainder of the mode line.
 
+@item %_
+Spaces sufficient to fill the remainder of the mode line.
+
 @item %%
 The character @samp{%}---this is how to include a literal @samp{%} in a
 string in which @code{%}-constructs are allowed.

=== modified file 'lisp/bindings.el'
--- lisp/bindings.el	2010-10-19 19:20:33 +0000
+++ lisp/bindings.el	2010-10-30 13:01:56 +0000
@@ -336,7 +336,8 @@
 	 'mode-line-modes
 	 `(which-func-mode ("" which-func-format ,spaces))
 	 `(global-mode-string ("" global-mode-string ,spaces))
-	 `(:eval (unless (display-graphic-p)
+	 `(:eval (if (display-graphic-p)
+		     ,(propertize " %_" 'help-echo help-echo)
 		   ,(propertize "-%-" 'help-echo help-echo)))))
        (standard-mode-line-modes
 	(list

=== modified file 'src/buffer.c'
--- src/buffer.c	2010-10-29 03:29:29 +0000
+++ src/buffer.c	2010-10-30 13:01:37 +0000
@@ -5571,6 +5571,7 @@
         remote machine.
   %[ -- print one [ for each recursive editing level.  %] similar.
   %% -- print %.   %- -- print infinitely many dashes.
+  %_ -- print infinitely many spaces.
 Decimal digits after the % specify field width to which to pad.  */);
 
   DEFVAR_LISP_NOPRO ("default-major-mode", &buffer_defaults.major_mode,

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2010-10-23 21:19:02 +0000
+++ src/xdisp.c	2010-10-30 12:37:24 +0000
@@ -19269,6 +19269,8 @@
 
 static char lots_of_dashes[] = "--------------------------------------------------------------------------------------------------------------------------------------------";
 
+static char lots_of_spaces[] = "                                                                                                                                            ";
+
 static const char *
 decode_mode_spec (struct window *w, register int c, int field_width,
 		  int precision, Lisp_Object *string)
@@ -19355,6 +19357,26 @@
 	  return lots_of_dashes;
       }
 
+    case '_':
+      {
+	register int i;
+
+	/* Let lots_of_spaces be a string of infinite length.  */
+	if (mode_line_target == MODE_LINE_NOPROP ||
+	    mode_line_target == MODE_LINE_STRING)
+	  return "  ";
+	if (field_width <= 0
+	    || field_width > sizeof (lots_of_spaces))
+	  {
+	    for (i = 0; i < FRAME_MESSAGE_BUF_SIZE (f) - 1; ++i)
+	      decode_mode_spec_buf[i] = ' ';
+	    decode_mode_spec_buf[i] = '\0';
+	    return decode_mode_spec_buf;
+	  }
+	else
+	  return lots_of_dashes;
+      }
+
     case 'b':
       obj = b->name;
       break;

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 7307@debbugs.gnu.org
> Date: Sat, 30 Oct 2010 15:27:32 +0200
> 
> On Sat, 30 Oct 2010 08:51:14 +0200 Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> From: Stephen Berman <stephen.berman@gmx.net>
> >> Date: Sat, 30 Oct 2010 01:05:32 +0200
> >> Cc: 
> >> 
> >> The only problem is specifying the length of the
> >> string: (window-width) as the maximum possible length fails here,
> >> because the mode line is constructed before the window-system frame, and
> >> has a value of just 10.  So I just picked 200 as a plausible maximum.
> >> Maybe there's a better way, but this seems to work ok.
> >
> > Would it help to use window-width as the maximum?
> 
> ?? See above.

Sorry, my bad.

> Anyway, maybe it's less kludgy to introduce a mode line
> %-construct for spaces.  Here's a patch for that, which just adapts the
> code for "%-".

IMO, this is cleaner.  Thanks.

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

On Sat, 30 Oct 2010 15:49:22 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>
>> Anyway, maybe it's less kludgy to introduce a mode line
>> %-construct for spaces.  Here's a patch for that, which just adapts the
>> code for "%-".
>
> IMO, this is cleaner.  Thanks.

There was a copy 'n' paste error in the patch, so here it is again, with
ChangeLog entries added.

Steve Berman


=== modified file 'doc/lispref/ChangeLog'
2010-10-30  Stephen Berman  <stephen.berman@gmx.net>

	* modes.texi (`%'-Constructs in the Mode Line): Document '%_'
	construct, which insert spaces at end of mode line (bug#7307).

=== modified file 'lisp/ChangeLog'
2010-10-30  Stephen Berman  <stephen.berman@gmx.net>

	* bindings.el: Use '%_' construct to insert spaces at end of mode
	line in graphical display (bug#7307).

=== modified file 'src/ChangeLog'
2010-10-30  Stephen Berman  <stephen.berman@gmx.net>

	* buffer.c (syms_of_buffer) <mode-line-format>: Document '%_'
	construct, which insert spaces at end of mode line (bug#7307).

	* xdisp.c (lots_of_spaces): New string variable.
	(decode_mode_spec): Add '%_' construct to insert spaces
	at end of mode line (bug#7307).

=== modified file 'doc/lispref/modes.texi'
--- doc/lispref/modes.texi	2010-08-22 19:30:26 +0000
+++ doc/lispref/modes.texi	2010-10-30 13:01:23 +0000
@@ -2029,6 +2029,9 @@
 @item %-
 Dashes sufficient to fill the remainder of the mode line.
 
+@item %_
+Spaces sufficient to fill the remainder of the mode line.
+
 @item %%
 The character @samp{%}---this is how to include a literal @samp{%} in a
 string in which @code{%}-constructs are allowed.

=== modified file 'lisp/bindings.el'
--- lisp/bindings.el	2010-10-19 19:20:33 +0000
+++ lisp/bindings.el	2010-10-30 13:01:56 +0000
@@ -336,7 +336,8 @@
 	 'mode-line-modes
 	 `(which-func-mode ("" which-func-format ,spaces))
 	 `(global-mode-string ("" global-mode-string ,spaces))
-	 `(:eval (unless (display-graphic-p)
+	 `(:eval (if (display-graphic-p)
+		     ,(propertize " %_" 'help-echo help-echo)
 		   ,(propertize "-%-" 'help-echo help-echo)))))
        (standard-mode-line-modes
 	(list

=== modified file 'src/buffer.c'
--- src/buffer.c	2010-10-29 03:29:29 +0000
+++ src/buffer.c	2010-10-30 13:01:37 +0000
@@ -5571,6 +5571,7 @@
         remote machine.
   %[ -- print one [ for each recursive editing level.  %] similar.
   %% -- print %.   %- -- print infinitely many dashes.
+  %_ -- print infinitely many spaces.
 Decimal digits after the % specify field width to which to pad.  */);
 
   DEFVAR_LISP_NOPRO ("default-major-mode", &buffer_defaults.major_mode,

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2010-10-23 21:19:02 +0000
+++ src/xdisp.c	2010-10-30 15:13:13 +0000
@@ -19269,6 +19269,8 @@
 
 static char lots_of_dashes[] = "--------------------------------------------------------------------------------------------------------------------------------------------";
 
+static char lots_of_spaces[] = "                                                                                                                                            ";
+
 static const char *
 decode_mode_spec (struct window *w, register int c, int field_width,
 		  int precision, Lisp_Object *string)
@@ -19355,6 +19357,26 @@
 	  return lots_of_dashes;
       }
 
+    case '_':
+      {
+	register int i;
+
+	/* Let lots_of_spaces be a string of infinite length.  */
+	if (mode_line_target == MODE_LINE_NOPROP ||
+	    mode_line_target == MODE_LINE_STRING)
+	  return "  ";
+	if (field_width <= 0
+	    || field_width > sizeof (lots_of_spaces))
+	  {
+	    for (i = 0; i < FRAME_MESSAGE_BUF_SIZE (f) - 1; ++i)
+	      decode_mode_spec_buf[i] = ' ';
+	    decode_mode_spec_buf[i] = '\0';
+	    return decode_mode_spec_buf;
+	  }
+	else
+	  return lots_of_spaces;
+      }
+
     case 'b':
       obj = b->name;
       break;

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stefan Monnier]

> just 10".)  Anyway, maybe it's less kludgy to introduce a mode line
> %-construct for spaces.  Here's a patch for that, which just adapts the

How 'bout something even simpler: make %- obey a config var which says
which char to use as filler.  That would default to - under a tty and
SPC on a GUI.


        Stefan

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

On Sat, 30 Oct 2010 23:54:53 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> just 10".)  Anyway, maybe it's less kludgy to introduce a mode line
>> %-construct for spaces.  Here's a patch for that, which just adapts the
>
> How 'bout something even simpler: make %- obey a config var which says
> which char to use as filler.  That would default to - under a tty and
> SPC on a GUI.

Hm, this may be conceptually simpler but at least for me not simpler to
implement, if I understand what you're suggesting, namely, that the
filler character should be provided by a defcustom, which is defined in
xdisp.c; is that right?  Before I try to do that, is the following
consistent with what you're suggesting (aside from lack the
configurability)?  If not, could you be more specific?

Steve Berman

=== modified file 'lisp/bindings.el'
--- lisp/bindings.el	2010-10-19 19:20:33 +0000
+++ lisp/bindings.el	2010-11-01 17:32:51 +0000
@@ -336,8 +336,7 @@
 	 'mode-line-modes
 	 `(which-func-mode ("" which-func-format ,spaces))
 	 `(global-mode-string ("" global-mode-string ,spaces))
-	 `(:eval (unless (display-graphic-p)
-		   ,(propertize "-%-" 'help-echo help-echo)))))
+	 `(:eval ,(propertize "%-" 'help-echo help-echo))))
        (standard-mode-line-modes
 	(list
 	 (propertize "%[" 'help-echo recursive-edit-help-echo)

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2010-10-23 21:19:02 +0000
+++ src/xdisp.c	2010-11-01 17:56:15 +0000
@@ -19267,8 +19267,15 @@
    Note we operate on the current buffer for most purposes,
    the exception being w->base_line_pos.  */
 
+static char lots_of_spaces[] =  "                                                                                                                                            ";
+
 static char lots_of_dashes[] = "--------------------------------------------------------------------------------------------------------------------------------------------";
 
+/* FIXME: This is wrong rather than test window-system, we should call
+   a new set-selection, which will then dispatch to x-set-selection, or
+   tty-set-selection, or w32-set-selection, ...  */
+EXFUN (Fwindow_system, 1);
+
 static const char *
 decode_mode_spec (struct window *w, register int c, int field_width,
 		  int precision, Lisp_Object *string)
@@ -19277,10 +19284,13 @@
   struct frame *f = XFRAME (WINDOW_FRAME (w));
   char *decode_mode_spec_buf = f->decode_mode_spec_buffer;
   struct buffer *b = current_buffer;
+  static char *mode_line_filler;
 
   obj = Qnil;
   *string = Qnil;
-
+  mode_line_filler = !NILP (Fwindow_system (Qnil)) ? lots_of_spaces 
+    : lots_of_dashes;
+  
   switch (c)
     {
     case '*':
@@ -19339,20 +19349,27 @@
       {
 	register int i;
 
-	/* Let lots_of_dashes be a string of infinite length.  */
+	/* Let mode_line_filler be a string of infinite length.  */
 	if (mode_line_target == MODE_LINE_NOPROP ||
 	    mode_line_target == MODE_LINE_STRING)
 	  return "--";
 	if (field_width <= 0
-	    || field_width > sizeof (lots_of_dashes))
+	    || field_width > sizeof (mode_line_filler))
 	  {
 	    for (i = 0; i < FRAME_MESSAGE_BUF_SIZE (f) - 1; ++i)
-	      decode_mode_spec_buf[i] = '-';
+	      if (!NILP (Fwindow_system (Qnil)))
+		{
+		  decode_mode_spec_buf[i] = ' ';
+		}
+	      else
+		{
+		  decode_mode_spec_buf[i] = '-';
+		}
 	    decode_mode_spec_buf[i] = '\0';
 	    return decode_mode_spec_buf;
 	  }
 	else
-	  return lots_of_dashes;
+	  return mode_line_filler;
       }
 
     case 'b':

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Mon, 01 Nov 2010 19:11:49 +0100
> Cc: 7307@debbugs.gnu.org
> 
> +  mode_line_filler = !NILP (Fwindow_system (Qnil)) ? lots_of_spaces 
> +    : lots_of_dashes;

Actually, you want to use `FRAME_WINDOW_P (f)' here.  It does exactly
what you mean, but with much smaller overhead (for starters, it
doesn't call Lisp).

> -	    || field_width > sizeof (lots_of_dashes))
> +	    || field_width > sizeof (mode_line_filler))

mode_line_filler is a pointer, so the result of sizeof here is not
what you want.

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

On Mon, 01 Nov 2010 21:32:12 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Date: Mon, 01 Nov 2010 19:11:49 +0100
>> Cc: 7307@debbugs.gnu.org
>> 
>> +  mode_line_filler = !NILP (Fwindow_system (Qnil)) ? lots_of_spaces 
>> +    : lots_of_dashes;
>
> Actually, you want to use `FRAME_WINDOW_P (f)' here.  It does exactly
> what you mean, but with much smaller overhead (for starters, it
> doesn't call Lisp).

Thanks. (I copied Fwindow_system, with the accompanying "This is wrong"
comment, from keyboard.c; I wonder why `FRAME_WINDOW_P (f)' isn't used
there.)

>> -	    || field_width > sizeof (lots_of_dashes))
>> +	    || field_width > sizeof (mode_line_filler))
>
> mode_line_filler is a pointer, so the result of sizeof here is not
> what you want.

Oops, thanks; I should be more careful with copy 'n' paste.

Thanks for the helpful comments.  I'll wait for Stefan's reply before
posting a corrected (or reworked) patch.

Steve Berman

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 7307@debbugs.gnu.org
> Date: Tue, 02 Nov 2010 00:35:25 +0100
> 
> On Mon, 01 Nov 2010 21:32:12 +0200 Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> From: Stephen Berman <stephen.berman@gmx.net>
> >> Date: Mon, 01 Nov 2010 19:11:49 +0100
> >> Cc: 7307@debbugs.gnu.org
> >> 
> >> +  mode_line_filler = !NILP (Fwindow_system (Qnil)) ? lots_of_spaces 
> >> +    : lots_of_dashes;
> >
> > Actually, you want to use `FRAME_WINDOW_P (f)' here.  It does exactly
> > what you mean, but with much smaller overhead (for starters, it
> > doesn't call Lisp).
> 
> Thanks. (I copied Fwindow_system, with the accompanying "This is wrong"
> comment, from keyboard.c; I wonder why `FRAME_WINDOW_P (f)' isn't used
> there.)

Indeed.  Chong, this is your change:

  2010-08-31  Chong Yidong  <cyd@stupidchicken.com>

	 * keyboard.c (command_loop_1): Don't call x-set-selection on tty.


Can you tell why you didn't use FRAME_WINDOW_P?  Am I missing some
subtlety?

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Eli Zaretskii]

> From: Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 02 Nov 2010 01:35:20 -0400
> Cc: 7307@debbugs.gnu.org
> Reply-To: Eli Zaretskii <eliz@gnu.org>
> 
>   2010-08-31  Chong Yidong  <cyd@stupidchicken.com>
> 
> 	 * keyboard.c (command_loop_1): Don't call x-set-selection on tty.
> 
> 
> Can you tell why you didn't use FRAME_WINDOW_P?  Am I missing some
> subtlety?

Actually, TRT here would be to use FRAME_TERMCAP_P (because of MSDOS
again).

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stefan Monnier]

> Hm, this may be conceptually simpler but at least for me not simpler to
> implement, if I understand what you're suggesting, namely, that the
> filler character should be provided by a defcustom, which is defined in
> xdisp.c; is that right?  Before I try to do that, is the following
> consistent with what you're suggesting (aside from lack the
> configurability)?  If not, could you be more specific?

Yes, that's pretty much what I meant, except for the lack
of configurability.  To make it configurable, we'll need to build the
lots_of_<foo> string dynamically in all cases.  Maybe that can be done
by removing the test of field_width.


        Stefan

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

On Tue, 02 Nov 2010 10:32:55 -0400 Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:

>> Hm, this may be conceptually simpler but at least for me not simpler to
>> implement, if I understand what you're suggesting, namely, that the
>> filler character should be provided by a defcustom, which is defined in
>> xdisp.c; is that right?  Before I try to do that, is the following
>> consistent with what you're suggesting (aside from lack the
>> configurability)?  If not, could you be more specific?
>
> Yes, that's pretty much what I meant, except for the lack
> of configurability.  To make it configurable, we'll need to build the
> lots_of_<foo> string dynamically in all cases.  Maybe that can be done
> by removing the test of field_width.

How about something like the following patch?  This builds the string
dynamically by filling decode_mode_spec_buf to the desired length,
dispensing with an additional lots_of_<foo> string.  AFAICT this gives
good results (though I admittedly haven't been able to find a case where
lots_of_filler (replacing lots_of_dashes) is used; what is such a
case?).

I also exposed the string character variable mode_line_filler to Lisp,
so it could be customizable, but I haven't been able to figure out how
to update the mode line with a new character.  Also, I don't know the
right way to set the default value: I assume it shouldn't be done in
decode_mode_spec() as below but at the top level together with the
DEFVAR, but then how can it be associated with the window whose mode
line is being constructed?

Steve Berman


*** /home/steve/bzr/emacs/trunk/src/xdisp.c	2010-10-24 13:56:16.000000000 +0200
--- /home/steve/bzr/emacs/quickfixes/src/xdisp.c	2010-11-05 16:02:21.000000000 +0100
***************
*** 642,647 ****
--- 642,652 ----
  
  int line_number_displayed;
  
+ /* Filler character used by the "%-" mode line spec.  Defaults to ' '
+    on a graphical display and to '-' on a tty.  */
+ 
+ static EMACS_INT mode_line_filler;
+ 
  /* Maximum buffer size for which to display line numbers.  */
  
  Lisp_Object Vline_number_display_limit;
***************
*** 19267,19274 ****
     Note we operate on the current buffer for most purposes,
     the exception being w->base_line_pos.  */
  
- static char lots_of_dashes[] = "--------------------------------------------------------------------------------------------------------------------------------------------";
- 
  static const char *
  decode_mode_spec (struct window *w, register int c, int field_width,
  		  int precision, Lisp_Object *string)
--- 19272,19277 ----
***************
*** 19277,19282 ****
--- 19280,19286 ----
    struct frame *f = XFRAME (WINDOW_FRAME (w));
    char *decode_mode_spec_buf = f->decode_mode_spec_buffer;
    struct buffer *b = current_buffer;
+   mode_line_filler = (FRAME_WINDOW_P (f)) ? ' ' : '-';
  
    obj = Qnil;
    *string = Qnil;
***************
*** 19338,19358 ****
      case '-':
        {
  	register int i;
! 
! 	/* Let lots_of_dashes be a string of infinite length.  */
! 	if (mode_line_target == MODE_LINE_NOPROP ||
! 	    mode_line_target == MODE_LINE_STRING)
! 	  return "--";
! 	if (field_width <= 0
! 	    || field_width > sizeof (lots_of_dashes))
! 	  {
! 	    for (i = 0; i < FRAME_MESSAGE_BUF_SIZE (f) - 1; ++i)
! 	      decode_mode_spec_buf[i] = '-';
! 	    decode_mode_spec_buf[i] = '\0';
! 	    return decode_mode_spec_buf;
! 	  }
! 	else
! 	  return lots_of_dashes;
        }
  
      case 'b':
--- 19342,19362 ----
      case '-':
        {
  	register int i;
! 	int fill_len;
! 	int lots_of_filler = 140 * sizeof (char);
!   
! 	/* Make a string of mode_line_filler characters.  */
! 	fill_len = 
! 	  (mode_line_target == MODE_LINE_NOPROP
! 	   || mode_line_target == MODE_LINE_STRING) 
! 	  ? 2
! 	  : ((field_width <= 0 || field_width > lots_of_filler)
! 	     ? FRAME_MESSAGE_BUF_SIZE (f) - 1 
! 	     : lots_of_filler);
! 	for (i = 0; i < fill_len; ++i)
! 	  decode_mode_spec_buf[i] = mode_line_filler;
! 	decode_mode_spec_buf[i] = '\0';
! 	return decode_mode_spec_buf;
        }
  
      case 'b':
***************
*** 26191,26196 ****
--- 26195,26204 ----
      doc: /* String (or mode line construct) included (normally) in `mode-line-format'.  */);
    Vglobal_mode_string = Qnil;
  
+   DEFVAR_INT ("mode-line-filler", &mode_line_filler,
+     doc: /* Filler character used by the "%-" mode line spec.
+ Defaults to ' ' on a graphical display and to '-' on a tty.  */);
+ 
    DEFVAR_LISP ("overlay-arrow-position", &Voverlay_arrow_position,
      doc: /* Marker for where to display an arrow on top of the buffer text.
  This must be the beginning of a line in order to work.

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Andreas Schwab]

Stephen Berman <stephen.berman@gmx.net> writes:

> --- 19342,19362 ----
>       case '-':
>         {
>   	register int i;
> ! 	int fill_len;
> ! 	int lots_of_filler = 140 * sizeof (char);

What is the point of sizeof (char)?

> ! 	/* Make a string of mode_line_filler characters.  */
> ! 	fill_len = 
> ! 	  (mode_line_target == MODE_LINE_NOPROP
> ! 	   || mode_line_target == MODE_LINE_STRING) 
> ! 	  ? 2
> ! 	  : ((field_width <= 0 || field_width > lots_of_filler)
> ! 	     ? FRAME_MESSAGE_BUF_SIZE (f) - 1 
> ! 	     : lots_of_filler);
> ! 	for (i = 0; i < fill_len; ++i)
> ! 	  decode_mode_spec_buf[i] = mode_line_filler;

What happens if mode_line_filler is a non-ASCII character?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

On Fri, 05 Nov 2010 19:45:54 +0100 Andreas Schwab <schwab@linux-m68k.org> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> --- 19342,19362 ----
>>       case '-':
>>         {
>>   	register int i;
>> ! 	int fill_len;
>> ! 	int lots_of_filler = 140 * sizeof (char);
>
> What is the point of sizeof (char)?

Oops, I was just confused. Thanks.

>
>> ! 	/* Make a string of mode_line_filler characters.  */
>> ! 	fill_len = 
>> ! 	  (mode_line_target == MODE_LINE_NOPROP
>> ! 	   || mode_line_target == MODE_LINE_STRING) 
>> ! 	  ? 2
>> ! 	  : ((field_width <= 0 || field_width > lots_of_filler)
>> ! 	     ? FRAME_MESSAGE_BUF_SIZE (f) - 1 
>> ! 	     : lots_of_filler);
>> ! 	for (i = 0; i < fill_len; ++i)
>> ! 	  decode_mode_spec_buf[i] = mode_line_filler;
>
> What happens if mode_line_filler is a non-ASCII character?

The mode line gets filled with a string of raw bytes :-(.  What is the
right thing to do here?

Steve Berman

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stefan Monnier]

> How about something like the following patch?

Looks pretty good.  It should probably come together with a Lisp part
that removes the current mode-line-format change.

> This builds the string
> dynamically by filling decode_mode_spec_buf to the desired length,
> dispensing with an additional lots_of_<foo> string.  AFAICT this gives
> good results (though I admittedly haven't been able to find a case where
> lots_of_filler (replacing lots_of_dashes) is used; what is such a
> case?).

I guess the lots_of_filler case is when you use things like "%5-".

> I also exposed the string character variable mode_line_filler to Lisp,
> so it could be customizable, but I haven't been able to figure out how
> to update the mode line with a new character.

That's because you re-initialize mode_line_filler each time.
Instead you should initialize it once and "for all" in init_xdisp.
But indeed, that exposes another problem: being global, you won't be
able to have "----" in tty frames and "    " in GUI frames in the
same process.

> Also, I don't know the right way to set the default value: I assume it
> shouldn't be done in decode_mode_spec() as below but at the top level
> together with the DEFVAR, but then how can it be associated with the
> window whose mode line is being constructed?

Indeed.  You could use a terminal property instead of (or additionally
to) a global variable, or you could let the global variable be a cons
cell where the car is the char to use for GUI frames and the cdr is for
tty frames :-(


        Stefan

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

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

On Mon, 08 Nov 2010 13:28:17 -0500 Stefan Monnier <monnier@iro.umontreal.ca> wrote:

(Sorry about the delay; I couldn't get back to it sooner.)

>> How about something like the following patch?
>
> Looks pretty good.  It should probably come together with a Lisp part
> that removes the current mode-line-format change.

See the attached patch.

>> This builds the string
>> dynamically by filling decode_mode_spec_buf to the desired length,
>> dispensing with an additional lots_of_<foo> string.  AFAICT this gives
>> good results (though I admittedly haven't been able to find a case where
>> lots_of_filler (replacing lots_of_dashes) is used; what is such a
>> case?).
>
> I guess the lots_of_filler case is when you use things like "%5-".

?? That's not a documented mode line %-spec, is it?  Anyway, using it I
still see no difference.

>> I also exposed the string character variable mode_line_filler to Lisp,
>> so it could be customizable, but I haven't been able to figure out how
>> to update the mode line with a new character.
>
> That's because you re-initialize mode_line_filler each time.
> Instead you should initialize it once and "for all" in init_xdisp.
> But indeed, that exposes another problem: being global, you won't be
> able to have "----" in tty frames and "    " in GUI frames in the
> same process.

I've discovered, if I'm not mistaken, that it apparently isn't necessary
to initialize mode_line_filler at the C level at all: rather, this can
be done in cus-start.el, and (except with --daemon, see below) it
correctly distinguishes tty and GUI frames.  (Admittedly, I don't
understand why it works, and would be grateful if you or someone else
could tell me.)

>> Also, I don't know the right way to set the default value: I assume it
>> shouldn't be done in decode_mode_spec() as below but at the top level
>> together with the DEFVAR, but then how can it be associated with the
>> window whose mode line is being constructed?
>
> Indeed.  You could use a terminal property instead of (or additionally
> to) a global variable, or you could let the global variable be a cons
> cell where the car is the char to use for GUI frames and the cdr is for
> tty frames :-(

It turns out to be much simpler (if I'm not mistaken).  That is, the
attached patch (against trunk from 2010-11-11) at least appears to DTRT,
except for three issues:

(i) As Andreas already noted, this does not work for non-ASCII
characters, which show up as octal codes.  I haven't yet figured out how
to fix this (I tried using string_make_multibyte, but then attempting to
use a non-ASCII character as the filler makes Emacs crash).

(ii) Using the character customization type and trying to set the filler
character to SPC signals the error "This field should contain a single
character".  I looked at the code but failed to see why this happens.
The patch below contains a workaround I found in ruler.el: allowing the
filler to choose between character and integer type; choosing the latter
with value 32 results in spaces filling the mode line.

(iii) Starting Emacs with --daemon and then opening a frame with
emacsclient -c results in the mode line being filled with `-'
characters, because the daemon uses a tty frame.  Moreover, the Custom
buffer for mode-line-filler says the state was "CHANGED outside
Customize", even with -Q.

If these three issues, or at least the first two, can be dealt with, do
you think this is a reasonable way to make the mode line filler
customizable?

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mode line filler patch --]
[-- Type: text/x-patch, Size: 3705 bytes --]

=== modified file 'lisp/bindings.el'
--- lisp/bindings.el	2010-10-19 19:20:33 +0000
+++ lisp/bindings.el	2010-11-19 11:27:06 +0000
@@ -336,8 +336,7 @@
 	 'mode-line-modes
 	 `(which-func-mode ("" which-func-format ,spaces))
 	 `(global-mode-string ("" global-mode-string ,spaces))
-	 `(:eval (unless (display-graphic-p)
-		   ,(propertize "-%-" 'help-echo help-echo)))))
+	 `(:eval ,(propertize "%-" 'help-echo help-echo))))
        (standard-mode-line-modes
 	(list
 	 (propertize "%[" 'help-echo recursive-edit-help-echo)

=== modified file 'lisp/cus-start.el'
--- lisp/cus-start.el	2010-10-31 22:47:12 +0000
+++ lisp/cus-start.el	2010-11-20 19:52:54 +0000
@@ -397,6 +397,15 @@
 	     (truncate-partial-width-windows display boolean "23.1")
 	     (mode-line-inverse-video mode-line boolean)
 	     (mode-line-in-non-selected-windows mode-line boolean "22.1")
+	     (mode-line-filler mode-line
+			       (choice
+				(character :tag "Character")
+				(integer :tag "Integer char value"
+					 ;; :validate ruler-mode-character-validate
+					 ))
+			       "24.1"
+			       :standard (if (display-graphic-p) ?  ?-)
+			       :initialize custom-initialize-delay)
 	     (line-number-display-limit display
 					(choice integer
 						(const :tag "No limit" nil)))

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2010-11-09 20:07:10 +0000
+++ src/xdisp.c	2010-11-19 22:13:02 +0000
@@ -667,6 +667,11 @@
 
 int cursor_type_changed;
 
+/* Filler character used by the "%-" mode line spec.  Defaults to ' '
+   on a graphical display and to '-' on a tty.  */
+
+EMACS_INT mode_line_filler;
+ 
 /* Nonzero after display_mode_line if %l was used and it displayed a
    line number.  */
 
@@ -19382,8 +19387,6 @@
    Note we operate on the current buffer for most purposes,
    the exception being w->base_line_pos.  */
 
-static char lots_of_dashes[] = "--------------------------------------------------------------------------------------------------------------------------------------------";
-
 static const char *
 decode_mode_spec (struct window *w, register int c, int field_width,
 		  int precision, Lisp_Object *string)
@@ -19453,21 +19456,21 @@
     case '-':
       {
 	register int i;
-
-	/* Let lots_of_dashes be a string of infinite length.  */
-	if (mode_line_target == MODE_LINE_NOPROP ||
-	    mode_line_target == MODE_LINE_STRING)
-	  return "--";
-	if (field_width <= 0
-	    || field_width > sizeof (lots_of_dashes))
-	  {
-	    for (i = 0; i < FRAME_MESSAGE_BUF_SIZE (f) - 1; ++i)
-	      decode_mode_spec_buf[i] = '-';
-	    decode_mode_spec_buf[i] = '\0';
-	    return decode_mode_spec_buf;
-	  }
-	else
-	  return lots_of_dashes;
+	int fill_len;
+	int lots_of_filler = 140;
+  
+	/* Make a string of mode_line_filler characters.  */
+	fill_len = 
+	  (mode_line_target == MODE_LINE_NOPROP
+	   || mode_line_target == MODE_LINE_STRING) 
+	  ? 2
+	  : ((field_width <= 0 || field_width > lots_of_filler)
+	     ? FRAME_MESSAGE_BUF_SIZE (f) - 1 
+	     : lots_of_filler);
+	for (i = 0; i < fill_len; ++i)
+	  decode_mode_spec_buf[i] = mode_line_filler;
+	decode_mode_spec_buf[i] = '\0';
+	return decode_mode_spec_buf;
       }
 
     case 'b':
@@ -26654,6 +26657,10 @@
     doc: /* String (or mode line construct) included (normally) in `mode-line-format'.  */);
   Vglobal_mode_string = Qnil;
 
+  DEFVAR_INT ("mode-line-filler", &mode_line_filler,
+    doc: /* Filler character used by the "%-" mode line spec.
+Defaults to ' ' on a graphical display and to '-' on a tty.  */);
+
   DEFVAR_LISP ("overlay-arrow-position", &Voverlay_arrow_position,
     doc: /* Marker for where to display an arrow on top of the buffer text.
 This must be the beginning of a line in order to work.

bug#7307: 24.0.50; Mode line had more than just dashes removed

[James Cloos]

>>>>> "SB" == Stephen Berman <stephen.berman@gmx.net> writes:

SB> not only removed the dashes at the end of the mode line, but also the
SB> help string in a tool tip together with the altered mouse pointer, which
SB> serves as a visual cue that the mode line is draggable (with vertically
SB> split windows).

It is still there, but is only one character wide.

(For me, it exists just after display-time's load average.)

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Stephen Berman]

On Sat, 30 Oct 2010 20:49:00 -0400 James Cloos <cloos@jhcloos.com> wrote:

>>>>>> "SB" == Stephen Berman <stephen.berman@gmx.net> writes:
>
> SB> not only removed the dashes at the end of the mode line, but also the
> SB> help string in a tool tip together with the altered mouse pointer, which
> SB> serves as a visual cue that the mode line is draggable (with vertically
> SB> split windows).
>
> It is still there, but is only one character wide.

True, but it's still disconcerting that it vanishes when you move the
mouse pointer further to the right along the mode line.

Steve Berman

bug#7307: 24.0.50; Mode line had more than just dashes removed

[Chong Yidong]

Stephen Berman <stephen.berman@gmx.net> writes:

> This change:
>
> 2010-10-18  Julien Danjou  <julien@danjou.info>
>
> 	* bindings.el: Remove end dashes in default mode-line-format.
>
> not only removed the dashes at the end of the mode line, but also the
> help string in a tool tip together with the altered mouse pointer, which
> serves as a visual cue that the mode line is draggable (with vertically
> split windows).

This is now fixed on trunk, as a side effect of a different change
(revision 108463).