unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#26909: 25.1; A face for margins
@ 2017-05-13 14:07 Yuri Khan
  2017-05-13 14:27 ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Yuri Khan @ 2017-05-13 14:07 UTC (permalink / raw)
  To: 26909

$ emacs -Q

Observed behavior: *scratch* buffer with two lines of text and an empty line.


M-x linum-mode RET

(Note: Linum is only used as a demonstration example. Other modes that
enable margins are affected similarly.)

Observed behavior: A margin appears on the left of the fringe. The
margin is white, the fringe is light gray. Line numbers 1, 2, and 3
are displayed in the margin.

Desired behavior: I would like to make the margin background the same
color as the fringe, and different from the default background.


M-: (set-face-attribute 'linum nil :inherit '(shadow fringe default)) RET

Observed behavior: Line numbers 1, 2, and 3 are displayed over light
gray background. Empty space below line number 3 is displayed in
default background color.


<up> <up> DEL

Observed behavior: Lines formerly numbered 1 and 2 are joined into a
long line. It is wrapped over so there is a gap between line numbers 1
and 2. The gap is displayed in default background color.


The last two actions demonstrate that customizing the faces of
whatever is displayed in the margin is not sufficient. A mechanism is
needed that will allow customization of the margin where nothing is
displayed. A new face would serve nicely.



In GNU Emacs 25.1.1 (x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2016-12-13, modified by Debian built on lgw01-55
Windowing system distributor 'The X.Org Foundation', version 11.0.11804000
System Description:    Ubuntu 16.04.2 LTS

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.1/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.1/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --with-x=yes --with-x-toolkit=gtk3
 --with-toolkit-scroll-bars 'CFLAGS=-g -O2 -fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_MONETARY: en_RU.UTF-8
  value of $LC_NUMERIC: en_RU.UTF-8
  value of $LC_TIME: en_RU.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  linum-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
Back to top level
Type C-x 1 to delete the help window.
mouse-2, RET: describe this symbol

nil [2 times]
C-c C-g is undefined
Quit [2 times]
Type C-x 1 to delete the help window, C-M-v to scroll help.
C-x <down> is undefined
Mark saved where search started [2 times]
scroll-down-command: Beginning of buffer

Load-path shadows:
None found.

Features:
(shadow sort mail-extr misearch multi-isearch emacsbug message dired
format-spec rfc822 mml mml-sec password-cache epg epg-config gnus-util
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils cl-extra thingatpt help-fns help-mode easymenu cl-loaddefs
pcase cl-lib debug linum time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 91665 11380)
 (symbols 48 19957 0)
 (miscs 40 93 221)
 (strings 32 15138 4640)
 (string-bytes 1 432034)
 (vectors 16 12162)
 (vector-slots 8 438633 9404)
 (floats 8 169 186)
 (intervals 56 736 16)
 (buffers 976 20)
 (heap 1024 47507 1126))





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2017-05-13 14:07 bug#26909: 25.1; A face for margins Yuri Khan
@ 2017-05-13 14:27 ` Eli Zaretskii
  2017-05-13 14:59   ` Yuri Khan
  2020-05-11 21:01   ` Clément Pit-Claudel
  0 siblings, 2 replies; 13+ messages in thread
From: Eli Zaretskii @ 2017-05-13 14:27 UTC (permalink / raw)
  To: Yuri Khan; +Cc: 26909

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Sat, 13 May 2017 21:07:03 +0700
> 
> The last two actions demonstrate that customizing the faces of
> whatever is displayed in the margin is not sufficient. A mechanism is
> needed that will allow customization of the margin where nothing is
> displayed. A new face would serve nicely.

A face can only affect places where something is displayed using that
face.  Display margins only display text if the buffer specifies text
properties or overlays which display in the margins.  But what you
would like to do calls for having a face that would affect screen
space where _nothing_ is displayed, and such screen space in Emacs is
always displayed using the frame's background color, not by using some
face.

You could specify a light gray background color for the frame, and
another color for the default face, but I think this would cause
unpleasant effects elsewhere on display, e.g. in the text area beyond
EOB.

IOW, I don't think introducing a new face would help here.  Some
additional mechanism would be necessary.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2017-05-13 14:27 ` Eli Zaretskii
@ 2017-05-13 14:59   ` Yuri Khan
  2017-05-13 16:40     ` Eli Zaretskii
  2020-05-11 21:01   ` Clément Pit-Claudel
  1 sibling, 1 reply; 13+ messages in thread
From: Yuri Khan @ 2017-05-13 14:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 26909

On Sat, May 13, 2017 at 9:27 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> A face can only affect places where something is displayed using that
> face.  Display margins only display text if the buffer specifies text
> properties or overlays which display in the margins.  But what you
> would like to do calls for having a face that would affect screen
> space where _nothing_ is displayed, and such screen space in Emacs is
> always displayed using the frame's background color, not by using some
> face.
>
> You could specify a light gray background color for the frame, and
> another color for the default face, but I think this would cause
> unpleasant effects elsewhere on display, e.g. in the text area beyond
> EOB.

That might in fact work for me. However, I do not seem to be able to do that.

(linum-mode) ^X^E
(set-frame-parameter nil 'background-color "gray95") ^X^E

* The backgrounds of both the margin and the buffer turn light gray.

(set-face-background 'default "white") ^X^E

* The backgrounds of both the buffer and the margin turn back to white.

Indeed, (elisp) Font and Color Parameters says:

| ‘background-color’
|      The color to use for the background of characters.  It is
|      equivalent to the ‘:background’ attribute of the ‘default’ face.

So the ‘default’ face is already special in that its :background is
used to draw the frame background. The proposed new face could be also
special that way.

(If by “background color for the frame” you meant something other than
the ‘background-color’ frame parameter, please point me to it.)





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2017-05-13 14:59   ` Yuri Khan
@ 2017-05-13 16:40     ` Eli Zaretskii
  2017-05-13 17:19       ` Yuri Khan
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2017-05-13 16:40 UTC (permalink / raw)
  To: Yuri Khan; +Cc: 26909

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Sat, 13 May 2017 21:59:29 +0700
> Cc: 26909@debbugs.gnu.org
> 
> > You could specify a light gray background color for the frame, and
> > another color for the default face, but I think this would cause
> > unpleasant effects elsewhere on display, e.g. in the text area beyond
> > EOB.
> 
> That might in fact work for me. However, I do not seem to be able to do that.
> 
> (linum-mode) ^X^E
> (set-frame-parameter nil 'background-color "gray95") ^X^E
> 
> * The backgrounds of both the margin and the buffer turn light gray.
> 
> (set-face-background 'default "white") ^X^E
> 
> * The backgrounds of both the buffer and the margin turn back to white.

Maybe I've misremembered, sorry.

Did you try remapping the default face, and if so, did that fail to
work as well?





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2017-05-13 16:40     ` Eli Zaretskii
@ 2017-05-13 17:19       ` Yuri Khan
  2017-05-13 17:42         ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Yuri Khan @ 2017-05-13 17:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 26909

On Sat, May 13, 2017 at 11:40 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> > You could specify a light gray background color for the frame, and
>> > another color for the default face, but I think this would cause
>> > unpleasant effects elsewhere on display, e.g. in the text area beyond
>> > EOB.
>>
>> (linum-mode) ^X^E
>> (set-frame-parameter nil 'background-color "gray95") ^X^E
>> (set-face-background 'default "white") ^X^E
>>
>> * The backgrounds of both the buffer and the margin turn back to white.
>
> Maybe I've misremembered, sorry.
>
> Did you try remapping the default face, and if so, did that fail to
> work as well?

You mean, like this?

(linum-mode)
(set-frame-parameter nil 'background-color "gray95")
(face-remap-add-relative 'default :background "white")

Same as above: remapping the default face’s background affects the margin, too.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2017-05-13 17:19       ` Yuri Khan
@ 2017-05-13 17:42         ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2017-05-13 17:42 UTC (permalink / raw)
  To: Yuri Khan; +Cc: 26909

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Sun, 14 May 2017 00:19:40 +0700
> Cc: 26909@debbugs.gnu.org
> 
> > Maybe I've misremembered, sorry.
> >
> > Did you try remapping the default face, and if so, did that fail to
> > work as well?
> 
> You mean, like this?
> 
> (linum-mode)
> (set-frame-parameter nil 'background-color "gray95")
> (face-remap-add-relative 'default :background "white")
> 
> Same as above: remapping the default face’s background affects the margin, too.

Sorry for my confusion.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2017-05-13 14:27 ` Eli Zaretskii
  2017-05-13 14:59   ` Yuri Khan
@ 2020-05-11 21:01   ` Clément Pit-Claudel
  2020-05-12 16:52     ` Eli Zaretskii
  2020-10-01  2:49     ` Lars Ingebrigtsen
  1 sibling, 2 replies; 13+ messages in thread
From: Clément Pit-Claudel @ 2020-05-11 21:01 UTC (permalink / raw)
  To: Eli Zaretskii, Yuri Khan; +Cc: 26909

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

Hi Eli and Yuri,

On 13/05/2017 10.27, Eli Zaretskii wrote:>> From: Yuri Khan <yuri.v.khan@gmail.com>
>> Date: Sat, 13 May 2017 21:07:03 +0700
>>
>> The last two actions demonstrate that customizing the faces of
>> whatever is displayed in the margin is not sufficient. A mechanism is
>> needed that will allow customization of the margin where nothing is
>> displayed. A new face would serve nicely.
> 
> A face can only affect places where something is displayed using that
> face.  Display margins only display text if the buffer specifies text
> properties or overlays which display in the margins.  But what you
> would like to do calls for having a face that would affect screen
> space where _nothing_ is displayed, and such screen space in Emacs is
> always displayed using the frame's background color, not by using some
> face.
> 
> IOW, I don't think introducing a new face would help here.  Some
> additional mechanism would be necessary.

A margin face would be great to have.  
How reasonable would it be to fill the margins with a stretched space?  Would it be too costly?
Currently we almost do that in extend_face_to_end_of_line, but short-circuits earlier in that function mean that this part is only applicable when there is e.g. a region.


  if (WINDOW_LEFT_MARGIN_WIDTH (it->w) > 0
      && it->glyph_row->used[LEFT_MARGIN_AREA] == 0)
    {
      it->glyph_row->glyphs[LEFT_MARGIN_AREA][0] = space_glyph;
      it->glyph_row->glyphs[LEFT_MARGIN_AREA][0].face_id =
        default_face->id;
      it->glyph_row->used[LEFT_MARGIN_AREA] = 1;
    }
  if (WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0
      && it->glyph_row->used[RIGHT_MARGIN_AREA] == 0)
    {
      it->glyph_row->glyphs[RIGHT_MARGIN_AREA][0] = space_glyph;
      it->glyph_row->glyphs[RIGHT_MARGIN_AREA][0].face_id =
        default_face->id;
      it->glyph_row->used[RIGHT_MARGIN_AREA] = 1;
    }

The (silly) attached patch confirms that removing the short-circuits makes it possible to set a face in the margins, but besides the performance aspect it only applies to lines that have contents.

Is this a reasonable way to go? If not, what might be better way?

Clément.

[-- Attachment #2: 0001-Add-a-margin-face-remove-short-circuits-in-extend_fa.patch --]
[-- Type: text/x-patch, Size: 3728 bytes --]

From a8a13cec225b00117557e5c84999a877843099da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Pit-Claudel?= <clement.pitclaudel@live.com>
Date: Mon, 11 May 2020 16:52:51 -0400
Subject: [PATCH] Add a margin face, remove short-circuits in
 extend_face_to_end_of_line

---
 src/dispextern.h |  1 +
 src/xdisp.c      | 10 ++++++----
 src/xfaces.c     |  3 +++
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index 0b1f3d14ae..2650d49e02 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1814,6 +1814,7 @@ #define FACE_UNIBYTE_P(FACE) ((FACE)->charset < 0)
   MODE_LINE_INACTIVE_FACE_ID,
   TOOL_BAR_FACE_ID,
   FRINGE_FACE_ID,
+  MARGIN_FACE_ID,
   HEADER_LINE_FACE_ID,
   SCROLL_BAR_FACE_ID,
   BORDER_FACE_ID,
diff --git a/src/xdisp.c b/src/xdisp.c
index 140d134572..5a7bf58828 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -21759,7 +21759,8 @@ extend_face_to_end_of_line (struct it *it)
       /* If the window has display margins, we will need to extend
 	 their face even if the text area is filled.  */
       && !(WINDOW_LEFT_MARGIN_WIDTH (it->w) > 0
-	   || WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0))
+	   || WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0)
+      && false)
     return;
 
   const int extend_face_id = (it->face_id == DEFAULT_FACE_ID
@@ -21785,7 +21786,8 @@ extend_face_to_end_of_line (struct it *it)
       && !face->stipple
 #endif
       && !it->glyph_row->reversed_p
-      && !Vdisplay_fill_column_indicator)
+      && !Vdisplay_fill_column_indicator
+      && false)
     return;
 
   /* Set the glyph row flag indicating that the face of the last glyph
@@ -21834,7 +21836,7 @@ extend_face_to_end_of_line (struct it *it)
 	    {
 	      it->glyph_row->glyphs[LEFT_MARGIN_AREA][0] = space_glyph;
 	      it->glyph_row->glyphs[LEFT_MARGIN_AREA][0].face_id =
-		default_face->id;
+		lookup_named_face (it->w, f, Qmargin, false);
 	      it->glyph_row->used[LEFT_MARGIN_AREA] = 1;
 	    }
 	  if (WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0
@@ -21842,7 +21844,7 @@ extend_face_to_end_of_line (struct it *it)
 	    {
 	      it->glyph_row->glyphs[RIGHT_MARGIN_AREA][0] = space_glyph;
 	      it->glyph_row->glyphs[RIGHT_MARGIN_AREA][0].face_id =
-		default_face->id;
+		lookup_named_face (it->w, f, Qmargin, false);
 	      it->glyph_row->used[RIGHT_MARGIN_AREA] = 1;
 	    }
 
diff --git a/src/xfaces.c b/src/xfaces.c
index bab142ade0..f604c43928 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4768,6 +4768,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
     case TAB_BAR_FACE_ID:		name = Qtab_bar;		break;
     case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
     case FRINGE_FACE_ID:		name = Qfringe;			break;
+    case MARGIN_FACE_ID:		name = Qmargin;			break;
     case SCROLL_BAR_FACE_ID:		name = Qscroll_bar;		break;
     case BORDER_FACE_ID:		name = Qborder;			break;
     case CURSOR_FACE_ID:		name = Qcursor;			break;
@@ -5463,6 +5464,7 @@ realize_basic_faces (struct frame *f)
       realize_named_face (f, Qmode_line_inactive, MODE_LINE_INACTIVE_FACE_ID);
       realize_named_face (f, Qtool_bar, TOOL_BAR_FACE_ID);
       realize_named_face (f, Qfringe, FRINGE_FACE_ID);
+      realize_named_face (f, Qmargin, MARGIN_FACE_ID);
       realize_named_face (f, Qheader_line, HEADER_LINE_FACE_ID);
       realize_named_face (f, Qscroll_bar, SCROLL_BAR_FACE_ID);
       realize_named_face (f, Qborder, BORDER_FACE_ID);
@@ -6808,6 +6810,7 @@ syms_of_xfaces (void)
   DEFSYM (Qtool_bar, "tool-bar");
   DEFSYM (Qtab_bar, "tab-bar");
   DEFSYM (Qfringe, "fringe");
+  DEFSYM (Qmargin, "margin");
   DEFSYM (Qtab_line, "tab-line");
   DEFSYM (Qheader_line, "header-line");
   DEFSYM (Qscroll_bar, "scroll-bar");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2020-05-11 21:01   ` Clément Pit-Claudel
@ 2020-05-12 16:52     ` Eli Zaretskii
  2020-05-12 17:06       ` Clément Pit-Claudel
  2020-10-01  2:49     ` Lars Ingebrigtsen
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2020-05-12 16:52 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: yuri.v.khan, 26909

> Cc: 26909@debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Mon, 11 May 2020 17:01:23 -0400
> 
> A margin face would be great to have.  
> How reasonable would it be to fill the margins with a stretched space?  Would it be too costly?

It would slow down redisplay, especially if the window is large, but
maybe the slow-down will not be so awful.

> The (silly) attached patch confirms that removing the short-circuits makes it possible to set a face in the margins, but besides the performance aspect it only applies to lines that have contents.
> 
> Is this a reasonable way to go?

Something like that, yes.  But you will need to make sure
extend_face_to_end_of_line is called also for empty lines.  And of
course the "silly" changes need to be made less silly.  And you need
to compute the pixel-width of the stretch glyph, since the code you
cited only places a single SPC character there, which is not what you
want if the margin is wider than one column.

> --- a/src/xfaces.c
> +++ b/src/xfaces.c
> @@ -4768,6 +4768,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
>      case TAB_BAR_FACE_ID:		name = Qtab_bar;		break;
>      case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
>      case FRINGE_FACE_ID:		name = Qfringe;			break;
> +    case MARGIN_FACE_ID:		name = Qmargin;			break;

If this is going to be an additional basic face, then why do you call
lookup_named_face  and not lookup_basic_face?

Also, this face should have a defface definition in faces.el.  (And
NEWS, and update for the manuals...)

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2020-05-12 16:52     ` Eli Zaretskii
@ 2020-05-12 17:06       ` Clément Pit-Claudel
  2020-05-12 17:21         ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Clément Pit-Claudel @ 2020-05-12 17:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yuri.v.khan, 26909

On 12/05/2020 12.52, Eli Zaretskii wrote:
>> Cc: 26909@debbugs.gnu.org
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Mon, 11 May 2020 17:01:23 -0400
>>
>> A margin face would be great to have.  
>> How reasonable would it be to fill the margins with a stretched space?  Would it be too costly?
> 
> It would slow down redisplay, especially if the window is large, but
> maybe the slow-down will not be so awful.

Got it.

> Something like that, yes.  But you will need to make sure
> extend_face_to_end_of_line is called also for empty lines.  And of
> course the "silly" changes need to be made less silly. 

Where the less silly strategy would be to only run the extend_face code when the margins face isn't customized, right?

> And you need
> to compute the pixel-width of the stretch glyph, since the code you
> cited only places a single SPC character there, which is not what you
> want if the margin is wider than one column.

Hmm, I think the code already does that? At least it seems to work with the patch I sent.

>> --- a/src/xfaces.c
>> +++ b/src/xfaces.c
>> @@ -4768,6 +4768,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
>>      case TAB_BAR_FACE_ID:		name = Qtab_bar;		break;
>>      case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
>>      case FRINGE_FACE_ID:		name = Qfringe;			break;
>> +    case MARGIN_FACE_ID:		name = Qmargin;			break;
> 
> If this is going to be an additional basic face, then why do you call
> lookup_named_face  and not lookup_basic_face?

It's because I have no idea what a basic face is, so I just cribbed from places that use the fringe face.  It does things like the following, and I have no idea what those mean:

      face_id = NILP (face) ? lookup_named_face (w, f, Qfringe, false)
	: lookup_derived_face (w, f, face, FRINGE_FACE_ID, 0);
      if (face_id < 0)
	face_id = FRINGE_FACE_ID;







^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2020-05-12 17:06       ` Clément Pit-Claudel
@ 2020-05-12 17:21         ` Eli Zaretskii
  2020-05-12 17:28           ` Clément Pit-Claudel
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2020-05-12 17:21 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: yuri.v.khan, 26909

> Cc: yuri.v.khan@gmail.com, 26909@debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Tue, 12 May 2020 13:06:00 -0400
> 
> > Something like that, yes.  But you will need to make sure
> > extend_face_to_end_of_line is called also for empty lines.  And of
> > course the "silly" changes need to be made less silly. 
> 
> Where the less silly strategy would be to only run the extend_face code when the margins face isn't customized, right?

The other way around, surely?

> > And you need
> > to compute the pixel-width of the stretch glyph, since the code you
> > cited only places a single SPC character there, which is not what you
> > want if the margin is wider than one column.
> 
> Hmm, I think the code already does that?

Does it where?  space_glyph is initiated in init_display_interactive,
and I don't see its pixel_width changed anywhere.  What do you get in
the width of that glyph in the margins?

> >> +    case MARGIN_FACE_ID:		name = Qmargin;			break;
> > 
> > If this is going to be an additional basic face, then why do you call
> > lookup_named_face  and not lookup_basic_face?
> 
> It's because I have no idea what a basic face is, so I just cribbed from places that use the fringe face.  It does things like the following, and I have no idea what those mean:
> 
>       face_id = NILP (face) ? lookup_named_face (w, f, Qfringe, false)
> 	: lookup_derived_face (w, f, face, FRINGE_FACE_ID, 0);
>       if (face_id < 0)
> 	face_id = FRINGE_FACE_ID;

Why not "crib" from a few lines above the code you hacked, where it
gets at the default face?





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2020-05-12 17:21         ` Eli Zaretskii
@ 2020-05-12 17:28           ` Clément Pit-Claudel
  2020-05-12 17:40             ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Clément Pit-Claudel @ 2020-05-12 17:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yuri.v.khan, 26909

On 12/05/2020 13.21, Eli Zaretskii wrote:
>> Cc: yuri.v.khan@gmail.com, 26909@debbugs.gnu.org
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Tue, 12 May 2020 13:06:00 -0400
>>
>>> Something like that, yes.  But you will need to make sure
>>> extend_face_to_end_of_line is called also for empty lines.  And of
>>> course the "silly" changes need to be made less silly. 
>>
>> Where the less silly strategy would be to only run the extend_face code when the margins face isn't customized, right?
> 
> The other way around, surely?

Indeed.

>>> And you need
>>> to compute the pixel-width of the stretch glyph, since the code you
>>> cited only places a single SPC character there, which is not what you
>>> want if the margin is wider than one column.
>>
>> Hmm, I think the code already does that?
> 
> Does it where?  space_glyph is initiated in init_display_interactive,
> and I don't see its pixel_width changed anywhere.  What do you get in
> the width of that glyph in the margins?
> 
>>>> +    case MARGIN_FACE_ID:		name = Qmargin;			break;
>>>
>>> If this is going to be an additional basic face, then why do you call
>>> lookup_named_face  and not lookup_basic_face?
>>
>> It's because I have no idea what a basic face is, so I just cribbed from places that use the fringe face.  It does things like the following, and I have no idea what those mean:
>>
>>       face_id = NILP (face) ? lookup_named_face (w, f, Qfringe, false)
>> 	: lookup_derived_face (w, f, face, FRINGE_FACE_ID, 0);
>>       if (face_id < 0)
>> 	face_id = FRINGE_FACE_ID;
> 
> Why not "crib" from a few lines above the code you hacked, where it
> gets at the default face?

Or from below, where it says "face_id = FRINGE_FACE_ID;"… I don't know: I don't know what this code does.  I will try to take the time to understand it; the original silly patch was just a few lines thrown together as a proof of concept.






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2020-05-12 17:28           ` Clément Pit-Claudel
@ 2020-05-12 17:40             ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2020-05-12 17:40 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: yuri.v.khan, 26909

> Cc: yuri.v.khan@gmail.com, 26909@debbugs.gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Tue, 12 May 2020 13:28:27 -0400
> 
> Or from below, where it says "face_id = FRINGE_FACE_ID;"… I don't know: I don't know what this code does.  I will try to take the time to understand it; the original silly patch was just a few lines thrown together as a proof of concept.

That code makes sure the face you need is "realized" (i.e., all of its
sources are merged) and cached in the frame's face cache for use by
the display code.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26909: 25.1; A face for margins
  2020-05-11 21:01   ` Clément Pit-Claudel
  2020-05-12 16:52     ` Eli Zaretskii
@ 2020-10-01  2:49     ` Lars Ingebrigtsen
  1 sibling, 0 replies; 13+ messages in thread
From: Lars Ingebrigtsen @ 2020-10-01  2:49 UTC (permalink / raw)
  To: Clément Pit-Claudel; +Cc: Yuri Khan, 26909

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:

> The (silly) attached patch confirms that removing the short-circuits
> makes it possible to set a face in the margins, but besides the
> performance aspect it only applies to lines that have contents.
>
> Is this a reasonable way to go? If not, what might be better way?

Skimming this thread, it seems the general agreement was that this would
be the way to go (but with some changes in how the faces were handled).
Clément, did you proceed further with this change?

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-10-01  2:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-13 14:07 bug#26909: 25.1; A face for margins Yuri Khan
2017-05-13 14:27 ` Eli Zaretskii
2017-05-13 14:59   ` Yuri Khan
2017-05-13 16:40     ` Eli Zaretskii
2017-05-13 17:19       ` Yuri Khan
2017-05-13 17:42         ` Eli Zaretskii
2020-05-11 21:01   ` Clément Pit-Claudel
2020-05-12 16:52     ` Eli Zaretskii
2020-05-12 17:06       ` Clément Pit-Claudel
2020-05-12 17:21         ` Eli Zaretskii
2020-05-12 17:28           ` Clément Pit-Claudel
2020-05-12 17:40             ` Eli Zaretskii
2020-10-01  2:49     ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).