Buffer-local faces

Buffer-local faces

[Miles Bader]

[I'm resending this message as a previous version seems to have been eaten by
 the email system somewhere ... @#!$% anti-spam/virus filters no doubt...]

I hacked up the buffer-local face implementation I talked about before.

The user-interface is a variable called `face-remappings':

   Alist of face mappings.
   Each element is of the form:
      (FACE . NEW-FACE)
   or
      (FACE NEW-FACE MERGE-FACE...),
   which causes NEW-FACE to be used where FACE normally would.
   If present, MERGE-FACE... are merged during display with NEW-FACE.

For instance, try evaluating the following:

  (set (make-local-variable 'face-remappings) '((default . italic)))

It (should) work with other special faces like like mode-line, &c, too.


Here's the patch:


M  src/xfaces.c
M  src/dispextern.h
M  src/fontset.c
M  src/xdisp.c

* modified files

*** orig/src/dispextern.h
--- mod/src/dispextern.h
***************
*** 2838,2843 ****
--- 2838,2844 ----
  int xstricmp P_ ((const unsigned char *, const unsigned char *));
  int lookup_face P_ ((struct frame *, Lisp_Object *, int, struct face *));
  int lookup_named_face P_ ((struct frame *, Lisp_Object, int));
+ int lookup_basic_face P_ ((struct frame *, int));
  int smaller_face P_ ((struct frame *, int, int));
  int face_with_height P_ ((struct frame *, int, int));
  int lookup_derived_face P_ ((struct frame *, Lisp_Object, int, int));
***************
*** 2854,2859 ****
--- 2855,2862 ----
  extern char unspecified_fg[], unspecified_bg[];
  void free_realized_multibyte_face P_ ((struct frame *, int));
  
+ extern Lisp_Object Vface_remappings;
+ 
  /* Defined in xfns.c  */
  
  #ifdef HAVE_X_WINDOWS


*** orig/src/fontset.c
--- mod/src/fontset.c
***************
*** 1252,1258 ****
        CHECK_NATNUM (ch);
        c = XINT (ch);
        f = XFRAME (selected_frame);
!       face_id = DEFAULT_FACE_ID;
      }
    else
      {
--- 1252,1258 ----
        CHECK_NATNUM (ch);
        c = XINT (ch);
        f = XFRAME (selected_frame);
!       face_id = lookup_basic_face (f, DEFAULT_FACE_ID);
      }
    else
      {


*** orig/src/xdisp.c
--- mod/src/xdisp.c
***************
*** 2028,2033 ****
--- 2028,2034 ----
       enum face_id base_face_id;
  {
    int highlight_region_p;
+   enum face_id remapped_base_face_id = base_face_id;
  
    /* Some precondition checks.  */
    xassert (w != NULL && it != NULL);
***************
*** 2044,2049 ****
--- 2045,2054 ----
        free_all_realized_faces (Qnil);
      }
  
+   /* Perhaps remap BASE_FACE_ID to a user-specified alternative.  */
+   if (! NILP (Vface_remappings))
+     remapped_base_face_id = lookup_basic_face (XFRAME (w->frame), base_face_id);
+ 
    /* Use one of the mode line rows of W's desired matrix if
       appropriate.  */
    if (row == NULL)
***************
*** 2059,2065 ****
    bzero (it, sizeof *it);
    it->current.overlay_string_index = -1;
    it->current.dpvec_index = -1;
!   it->base_face_id = base_face_id;
    it->string = Qnil;
    IT_STRING_CHARPOS (*it) = IT_STRING_BYTEPOS (*it) = -1;
  
--- 2064,2070 ----
    bzero (it, sizeof *it);
    it->current.overlay_string_index = -1;
    it->current.dpvec_index = -1;
!   it->base_face_id = remapped_base_face_id;
    it->string = Qnil;
    IT_STRING_CHARPOS (*it) = IT_STRING_BYTEPOS (*it) = -1;
  
***************
*** 2243,2253 ****
      {
        struct face *face;
  
!       it->face_id = base_face_id;
  
        /* If we have a boxed mode line, make the first character appear
  	 with a left box line.  */
!       face = FACE_FROM_ID (it->f, base_face_id);
        if (face->box != FACE_NO_BOX)
  	it->start_of_box_run_p = 1;
  #ifdef HAVE_WINDOW_SYSTEM
--- 2248,2258 ----
      {
        struct face *face;
  
!       it->face_id = remapped_base_face_id;
  
        /* If we have a boxed mode line, make the first character appear
  	 with a left box line.  */
!       face = FACE_FROM_ID (it->f, remapped_base_face_id);
        if (face->box != FACE_NO_BOX)
  	it->start_of_box_run_p = 1;
  #ifdef HAVE_WINDOW_SYSTEM
***************
*** 3491,3497 ****
  	      /* Value is a multiple of the canonical char height.  */
  	      struct face *face;
  
! 	      face = FACE_FROM_ID (it->f, DEFAULT_FACE_ID);
  	      new_height = (XFLOATINT (it->font_height)
  			    * XINT (face->lface[LFACE_HEIGHT_INDEX]));
  	    }
--- 3496,3503 ----
  	      /* Value is a multiple of the canonical char height.  */
  	      struct face *face;
  
! 	      face = FACE_FROM_ID (it->f,
! 				   lookup_basic_face (it->f, DEFAULT_FACE_ID));
  	      new_height = (XFLOATINT (it->font_height)
  			    * XINT (face->lface[LFACE_HEIGHT_INDEX]));
  	    }
***************
*** 3591,3597 ****
  	      || EQ (XCAR (prop), Qright_fringe))
  	  && CONSP (XCDR (prop)))
  	{
! 	  unsigned face_id = DEFAULT_FACE_ID;
  
  	  /* Save current settings of IT so that we can restore them
  	     when we are finished with the glyph property value.  */
--- 3597,3603 ----
  	      || EQ (XCAR (prop), Qright_fringe))
  	  && CONSP (XCDR (prop)))
  	{
! 	  unsigned face_id = lookup_basic_face (it->f, DEFAULT_FACE_ID);
  
  	  /* Save current settings of IT so that we can restore them
  	     when we are finished with the glyph property value.  */


*** orig/src/xfaces.c
--- mod/src/xfaces.c
***************
*** 400,405 ****
--- 400,412 ----
  
  Lisp_Object Vface_new_frame_defaults;
  
+ /* Alist of face mappings.  Each element is either of the form
+    (FACE . NEW-FACE), or (FACE NEW-FACE MERGE-FACE...),
+    where FACE is the named used for lookups, and NEW-FACE is the name
+    that actually gets looked up.  If present, MERGE-FACE... are merged
+    during display of FACE, with NEW-FACE.  */
+ Lisp_Object Vface_remappings;
+ 
  /* The next ID to assign to Lisp faces.  */
  
  static int next_lface_id;
***************
*** 3196,3213 ****
     for another face, return that face's definition.  If SIGNAL_P is
     non-zero, signal an error if FACE_NAME is not a valid face name.
     If SIGNAL_P is zero, value is nil if FACE_NAME is not a valid face
!    name.  */
  
  static INLINE Lisp_Object
! lface_from_face_name (f, face_name, signal_p)
       struct frame *f;
       Lisp_Object face_name;
       int signal_p;
  {
    Lisp_Object lface;
  
    face_name = resolve_face_name (face_name);
  
    if (f)
      lface = assq_no_quit (face_name, f->face_alist);
    else
--- 3203,3271 ----
     for another face, return that face's definition.  If SIGNAL_P is
     non-zero, signal an error if FACE_NAME is not a valid face name.
     If SIGNAL_P is zero, value is nil if FACE_NAME is not a valid face
!    name.  If ATTRS is non-zero, then it is a lisp-face vector, and
!    successful lookups copy the lface vector into it, _along with any
!    merges from face mappings_ (so in that cases, the returned lisp
!    vector and the values in ATTRS may not be identical).  CYCLE_CHECK
!    is used internally to detect loops in face mapping; it can be Qnil
!    when called from other places that are not involved in mutual
!    recursion with this function.  */
  
  static INLINE Lisp_Object
! lface_from_face_name_with_attrs (f, face_name, attrs, signal_p, cycle_check)
       struct frame *f;
       Lisp_Object face_name;
+      Lisp_Object *attrs;
       int signal_p;
+      Lisp_Object cycle_check;
  {
    Lisp_Object lface;
+   Lisp_Object face_remapping;
  
    face_name = resolve_face_name (face_name);
  
+   /* See if SYMBOL has been remapped to some other face (usually this
+      is done buffer-locally).  */
+   face_remapping = assq_no_quit (face_name, Vface_remappings);
+   if (! NILP (face_remapping))
+     {
+       /* Make sure we're not in an mapping loop.  */
+       cycle_check = CYCLE_CHECK (cycle_check, face_name, 15);
+ 
+       if (! NILP (cycle_check))
+ 	{
+ 	  /* No cycle detected, lookup FACE_NAME's mapping instead.  */
+ 	  Lisp_Object merges;
+ 
+ 	  face_remapping = XCDR (face_remapping);
+ 
+ 	  /* A mapping may also contain a list of `merge faces', which
+ 	     we ignore in this function.  */
+ 	  if (CONSP (face_remapping) && SYMBOLP (XCAR (face_remapping)))
+ 	    {
+ 	      merges = XCDR (face_remapping);
+ 	      face_remapping = XCAR (face_remapping);
+ 	    }
+ 	  else
+ 	    merges = Qnil;
+ 
+ 	  /* Recursively lookup FACE_REMAPPING, if it's not obviously bogus.  */
+ 	  if (SYMBOLP (face_remapping)
+ 	      && !NILP (face_remapping)
+ 	      && !EQ (face_remapping, face_name))
+ 	    {
+ 	      lface = lface_from_face_name_with_attrs (f, face_remapping, attrs,
+ 						       signal_p, cycle_check);
+ 
+ 	      /* Merge in additional faces specified in the mapping.  */
+ 	      if (attrs && !NILP (lface) && !NILP (merges))
+ 		merge_face_inheritance (f, merges, attrs, cycle_check);
+ 
+ 	      return lface;
+ 	    }
+ 	}
+     }
+ 
    if (f)
      lface = assq_no_quit (face_name, f->face_alist);
    else
***************
*** 3219,3227 ****
--- 3277,3306 ----
      signal_error ("Invalid face", face_name);
  
    check_lface (lface);
+ 
+   if (attrs)
+     bcopy (XVECTOR (lface)->contents, attrs, LFACE_VECTOR_SIZE * sizeof *attrs);
+ 
    return lface;
  }
  
+ /* Return the face definition of FACE_NAME on frame F.  F null means
+    return the definition for new frames.  FACE_NAME may be a string or
+    a symbol (apparently Emacs 20.2 allowed strings as face names in
+    face text properties; Ediff uses that).  If FACE_NAME is an alias
+    for another face, return that face's definition.  If SIGNAL_P is
+    non-zero, signal an error if FACE_NAME is not a valid face name.
+    If SIGNAL_P is zero, value is nil if FACE_NAME is not a valid face
+    name.  */
+ static INLINE Lisp_Object
+ lface_from_face_name (f, face_name, signal_p)
+      struct frame *f;
+      Lisp_Object face_name;
+      int signal_p;
+ {
+   return lface_from_face_name_with_attrs (f, face_name, 0, signal_p, Qnil);
+ }
+ 
  
  /* Get face attributes of face FACE_NAME from frame-local faces on
     frame F.  Store the resulting attributes in ATTRS which must point
***************
*** 3237,3255 ****
       int signal_p;
  {
    Lisp_Object lface;
!   int success_p;
! 
!   lface = lface_from_face_name (f, face_name, signal_p);
!   if (!NILP (lface))
!     {
!       bcopy (XVECTOR (lface)->contents, attrs,
! 	     LFACE_VECTOR_SIZE * sizeof *attrs);
!       success_p = 1;
!     }
!   else
!     success_p = 0;
! 
!   return success_p;
  }
  
  
--- 3316,3323 ----
       int signal_p;
  {
    Lisp_Object lface;
!   lface = lface_from_face_name_with_attrs (f, face_name, attrs, signal_p, Qnil);
!   return !NILP (lface);
  }
  
  
***************
*** 5779,5789 ****
     face couldn't be determined, which might happen if the default face
     isn't realized and cannot be realized.  */
  
! int
! lookup_named_face (f, symbol, c)
       struct frame *f;
       Lisp_Object symbol;
       int c;
  {
    Lisp_Object attrs[LFACE_VECTOR_SIZE];
    Lisp_Object symbol_attrs[LFACE_VECTOR_SIZE];
--- 5847,5858 ----
     face couldn't be determined, which might happen if the default face
     isn't realized and cannot be realized.  */
  
! static int
! lookup_named_face_1 (f, symbol, c, signal_p)
       struct frame *f;
       Lisp_Object symbol;
       int c;
+      int signal_p;
  {
    Lisp_Object attrs[LFACE_VECTOR_SIZE];
    Lisp_Object symbol_attrs[LFACE_VECTOR_SIZE];
***************
*** 5796,5807 ****
        default_face = FACE_FROM_ID (f, DEFAULT_FACE_ID);
      }
  
!   get_lface_attributes (f, symbol, symbol_attrs, 1);
    bcopy (default_face->lface, attrs, sizeof attrs);
    merge_face_vectors (f, symbol_attrs, attrs, Qnil);
    return lookup_face (f, attrs, c, NULL);
  }
  
  
  /* Return the ID of the realized ASCII face of Lisp face with ID
     LFACE_ID on frame F.  Value is -1 if LFACE_ID isn't valid.  */
--- 5865,5933 ----
        default_face = FACE_FROM_ID (f, DEFAULT_FACE_ID);
      }
  
!   if (! get_lface_attributes (f, symbol, symbol_attrs, signal_p))
!     return -1;
! 
    bcopy (default_face->lface, attrs, sizeof attrs);
    merge_face_vectors (f, symbol_attrs, attrs, Qnil);
+ 
    return lookup_face (f, attrs, c, NULL);
  }
  
+ /* Return the face id of the realized face for named face SYMBOL on
+    frame F suitable for displaying character C.  Value is -1 if the
+    face couldn't be determined, which might happen if the default face
+    isn't realized and cannot be realized.  */
+ 
+ int
+ lookup_named_face (f, symbol, c)
+      struct frame *f;
+      Lisp_Object symbol;
+      int c;
+ {
+   return lookup_named_face_1 (f, symbol, c);
+ }
+ 
+ 
+ /* Return the display face-id of the basic face who's canonical face-id
+    is FACE_ID.  The return value will usually simply be FACE_ID, unless that
+    basic face has bee remapped via Vface_remappings.  This function is
+    conservative: if something goes wrong, it will simply return FACE_ID
+    rather than signal an error.   */
+ 
+ int
+ lookup_basic_face (f, face_id)
+      struct frame *f;
+      int face_id;
+ {
+   Lisp_Object name, mapping;
+   int remapped_face_id;
+ 
+   if (NILP (Vface_remappings))
+     return face_id;		/* Nothing to do.  */
+ 
+   switch (face_id)
+     {
+     case DEFAULT_FACE_ID:		name = Qdefault;		break;
+     case MODE_LINE_FACE_ID:		name = Qmode_line;		break;
+     case MODE_LINE_INACTIVE_FACE_ID:	name = Qmode_line_inactive;	break;
+     case HEADER_LINE_FACE_ID:		name = Qheader_line;		break;
+ 
+     default:
+       return face_id;		/* Give up.  */
+     }
+ 
+   mapping = assq_no_quit (name, Vface_remappings);
+   if (NILP (mapping))
+     return face_id;		/* Give up.  */
+ 
+   remapped_face_id = lookup_named_face_1 (f, name, 0, 0);
+   if (remapped_face_id < 0)
+     return face_id;		/* Give up. */
+ 
+   return remapped_face_id;
+ }
+ 
  
  /* Return the ID of the realized ASCII face of Lisp face with ID
     LFACE_ID on frame F.  Value is -1 if LFACE_ID isn't valid.  */
***************
*** 7372,7384 ****
  
    *endptr = endpos;
  
!   default_face = FACE_FROM_ID (f, DEFAULT_FACE_ID);
  
    /* Optimize common cases where we can use the default face.  */
    if (noverlays == 0
        && NILP (prop)
        && !(pos >= region_beg && pos < region_end))
!     return DEFAULT_FACE_ID;
  
    /* Begin with attributes from the default face.  */
    bcopy (default_face->lface, attrs, sizeof attrs);
--- 7498,7515 ----
  
    *endptr = endpos;
  
! 
!   /* Perhaps remap BASE_FACE_ID to a user-specified alternative.  */
!   if (NILP (Vface_remappings))
!     default_face = FACE_FROM_ID (f, DEFAULT_FACE_ID);
!   else
!     default_face = FACE_FROM_ID (f, lookup_basic_face (f, DEFAULT_FACE_ID));
  
    /* Optimize common cases where we can use the default face.  */
    if (noverlays == 0
        && NILP (prop)
        && !(pos >= region_beg && pos < region_end))
!     return default_face->id;
  
    /* Begin with attributes from the default face.  */
    bcopy (default_face->lface, attrs, sizeof attrs);
***************
*** 7839,7844 ****
--- 7970,7985 ----
  ignore.  */);
    Vface_ignored_fonts = Qnil;
  
+   DEFVAR_LISP ("face-remappings", &Vface_remappings,
+ 	       doc: /* Alist of face mappings.
+ Each element is of the form:
+    (FACE . NEW-FACE)
+ or
+    (FACE NEW-FACE MERGE-FACE...),
+ which causes NEW-FACE to be used where FACE normally would.
+ If present, MERGE-FACE... are merged during display with NEW-FACE.  */);
+   Vface_remappings = Qnil;
+ 
    DEFVAR_LISP ("face-font-rescale-alist", &Vface_font_rescale_alist,
  	       doc: /* Alist of fonts vs the rescaling factors.
  Each element is a cons (FONT-NAME-PATTERN . RESCALE-RATIO), where



-Miles
-- 
We are all lying in the gutter, but some of us are looking at the stars.
-Oscar Wilde

Re: Buffer-local faces

[Miles Bader]

On Mon, May 03, 2004 at 09:03:50AM -0400, Miles Bader wrote:
> [I'm resending this message as a previous version seems to have been eaten by
>  the email system somewhere ... @#!$% anti-spam/virus filters no doubt...]

... and of course, right on schedule, up shows the original message.

Sigh (and sorry).

-Miles
-- 
/\ /\
(^.^)
(")")
*This is the cute kitty virus, please copy this into your sig so it can spread.

Re: Buffer-local faces

[Richard Stallman]

       Each element is of the form:
	  (FACE . NEW-FACE)
       or
	  (FACE NEW-FACE MERGE-FACE...),
       which causes NEW-FACE to be used where FACE normally would.
       If present, MERGE-FACE... are merged during display with NEW-FACE.

Wouldn't it be cleaner just to have one kind of element,
(FACE REPLACEMENT-FACES...)

Re: Buffer-local faces

[Stefan Monnier]

>       (FACE NEW-FACE MERGE-FACE...),
>    which causes NEW-FACE to be used where FACE normally would.
>    If present, MERGE-FACE... are merged during display with NEW-FACE.

How often would you need MERGE-FACE(s) ?  I ask because it seems unnecessary
since you can also use a new face with a :inherit slot instead, right?


        Stefan

Re: Buffer-local faces

[Miles Bader]

On Mon, May 03, 2004 at 06:20:46PM -0400, Richard Stallman wrote:
>        Each element is of the form:
> 	  (FACE . NEW-FACE)
>        or
> 	  (FACE NEW-FACE MERGE-FACE...),
>        which causes NEW-FACE to be used where FACE normally would.
>        If present, MERGE-FACE... are merged during display with NEW-FACE.
> 
> Wouldn't it be cleaner just to have one kind of element,
> (FACE REPLACEMENT-FACES...)

I think so, but I'm not entirely sure if it's possible; perhaps it is.

The question is whether there are places that want remapping, but don't
operate in terms of face-ids -- e.g., they use `lisp-faces' (lisp vectors),
or face names.  Face-ids can represent merged faces, but the other
representations can not, so my original thought while implementing this was
that I'd need to support the concept of a `simple remapping' that merely
returned one face name/vector for another.

However that may actually not be necessary.  I still have to go through the
corner cases I think.

A related questions is whether the remapping should be visible to lisp code
using face functions.  For instance, does (face-attribute 'default :family)
1. return the remapped family, or 2. return the family of the `underlying'
default face?

My current patch does the former, but now I'm leaning towards making it the
latter, and having remapping be a pure display operation -- it just seems
simpler to deal with.  Lisp code that's interested in such matters can look
at the variable to see what's up.

-Miles
-- 
`Cars give people wonderful freedom and increase their opportunities.
 But they also destroy the environment, to an extent so drastic that
 they kill all social life' (from _A Pattern Language_)

Re: Buffer-local faces

[Miles Bader]

On Mon, May 03, 2004 at 06:42:10PM -0400, Stefan Monnier wrote:
> >       (FACE NEW-FACE MERGE-FACE...),
> >    which causes NEW-FACE to be used where FACE normally would.
> >    If present, MERGE-FACE... are merged during display with NEW-FACE.
> 
> How often would you need MERGE-FACE(s) ?  I ask because it seems unnecessary
> since you can also use a new face with a :inherit slot instead, right?

My thought was that this would be convenient for people that want to just
tweak faces in a mode-hook, and allow them to do it without making a new
face.  It's annoying to always have to name everything.  Implementation-wise
I don't think there's any real cost to allowing it (the code to merge from a
list of faces is already there for the use of :inherit).

Actually, this doesn't work currently (not sure why), but I thought a nice
use would be something like (region region bold) -- which would _extend_ the
region face to also boldify stuff, as opposed to (region bold) which would
_replace_ the region face.  Allowing this source of `mixin' face would be
very handy for users I think.

-Miles
-- 
`The suburb is an obsolete and contradictory form of human settlement'

Re: Buffer-local faces

[Juri Linkov]

Miles Bader <miles@gnu.org> writes:
> Actually, this doesn't work currently (not sure why), but I thought a nice
> use would be something like (region region bold) -- which would _extend_ the
> region face to also boldify stuff, as opposed to (region bold) which would
> _replace_ the region face.  Allowing this source of `mixin' face would be
> very handy for users I think.

Then subtracting face attributes would be useful too.  For example,
using the `-' symbol before the face name could remove its attributes
from the composed face, e.g. using (region region - bold) to remove
all attributes of the `bold' face whose values are equal to attribute
values of the `region' face.  This may complicate things, but this
is no less useful than merging faces.

And using (setq face-remapping-alist '((default - bold))) could remove
the `bold' attributes from all faces.  I don't know if this can be
easily added to your current patch since it requires iterating over
all faces and removing attributes from faces inheriting from the face
in key position of every element of `face-remapping-alist'.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Buffer-local faces

[Eli Zaretskii]

> Date: Mon, 3 May 2004 19:19:27 -0400
> From: Miles Bader <miles@gnu.org>
> 
> My current patch does the former, but now I'm leaning towards making it the
> latter, and having remapping be a pure display operation -- it just seems
> simpler to deal with.  Lisp code that's interested in such matters can look
> at the variable to see what's up.

Or we could add a new function, face-real-attribute, say, that would
return the attribute after any remappings took place.  For example, it
might be useful to know what is the real color displayed on
color-challenged terminals when the face requests a color that isn't
directly available.

Re: Buffer-local faces

[Miles Bader]

On Tue, May 04, 2004 at 08:45:21AM +0300, Juri Linkov wrote:
> Then subtracting face attributes would be useful too.  For example,
> using the `-' symbol before the face name could remove its attributes
> from the composed face, e.g. using (region region - bold) to remove
> all attributes of the `bold' face whose values are equal to attribute
> values of the `region' face.  This may complicate things, but this
> is no less useful than merging faces.

It's a much less obvious operation (what does subtracting red from black
yield?  [Ok, maybe cyan :-]) and I can't offhand think of any obvious uses
for it....

Morever, I don't think would help in this case because face-merging more
specific faces during redisplay will always override the default attributes.

A _different_ feature that might work is a way to mark a face attribute as
`non overridable'.  For instance if a face attribute could be a list like
(fixed VALUE), where VALUE is a normal value for that attribute, face
merging would always use the `base' value for that attribute instead of the
`normally overriding' value.

[Since the face-remapping code just uses normal face merging, this feature
would work with that too, but it wouldn't be specific to it.]

Generally useful?  I'm not sure, but it could probably handle the `no bold
please' case.

-Miles
-- 
"Whatever you do will be insignificant, but it is very important that
 you do it."  Mahatma Ghandi

Re: Buffer-local faces

[Kim F. Storm]

Miles Bader <miles@gnu.org> writes:

> On Tue, May 04, 2004 at 11:18:21AM +0200, David Kastrup wrote:
> > I am uncomfortable about the whole change.  And the reason has to do
> > with the feature freeze.  Now you may argue that the change is not so
> > intrusive as to be likely to trigger new bugs, but that's beside the
> > point.
>
> I said nothing about the feature freeze.  I did not post my patch to `sneak
> in under the wire' of the freeze, I posted it because I (1) happened to have
> been working on it, and (2) came up with something nice.

Again, feature freeze means that people should switch attention away
from developing new features -- but that doesn't exclude that we
can add things (for a short period) that people have already been
working on before the freeze.

If those new features are useful (and the changes are not too
intrusive), I would prefer to get them into 21.5 rather than wait for
22.x to come out (history shows that major emacs releases don't happen
everyday :-| )

I think Miles' approach is quite elegant, as it solves a real problem
in a simple and IMO clean way.  There may be cases that are not
handled by this approach, but it is still a whole lot better than
having no approach at all.

And even if we find a better or more general approach later on, I
believe it can co-exist with Miles' code, as that code works on a
pretty low level (implementation-wise).

Of course, there may be things that don't work with Miles' patch (fringe
faces may be one case -- haven't checked), but it could be fixed during
pretest if deemed necessary.

I would love to have this feature.

> I don't think it's a kludge at all, it's an elegant way of leveraging emacs'
> very flexible variable mechanism to achieve the goal -- not only is it almost
> trivial to implement, but it would seem to fit very well with the way emacs
> modes work.

I don't think it is a kludge either.

>
> Perhaps xemacs has a better way of doing it, I don't know -- I'm afraid I'm
> not familiar with xemacs in recent years.

As a true GNU emacs evangelist, I've never been familiar with xemacs...

--
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Buffer-local faces

[David Kastrup]

Miles Bader <miles@gnu.org> writes:

> On Mon, May 03, 2004 at 06:42:10PM -0400, Stefan Monnier wrote:
> > >       (FACE NEW-FACE MERGE-FACE...),
> > >    which causes NEW-FACE to be used where FACE normally would.
> > >    If present, MERGE-FACE... are merged during display with NEW-FACE.
> > 
> > How often would you need MERGE-FACE(s) ?  I ask because it seems
> > unnecessary since you can also use a new face with a :inherit slot
> > instead, right?
> 
> My thought was that this would be convenient for people that want to
> just tweak faces in a mode-hook, and allow them to do it without
> making a new face.  It's annoying to always have to name everything.
> Implementation-wise I don't think there's any real cost to allowing
> it (the code to merge from a list of faces is already there for the
> use of :inherit).

I am uncomfortable about the whole change.  And the reason has to do
with the feature freeze.  Now you may argue that the change is not so
intrusive as to be likely to trigger new bugs, but that's beside the
point.

The first question to resolve with regard to a new feature is to
figure out how to do it right, not how to circumnavigate the feature
freeze best.  If we base a decision like this on criteria like a
feature freeze, then it may come back to haunt us at a later time.

So I think we should first try to resolve what the perceived problem
is, what extensions or generalizations of this problem should also be
solvable, and what would be the most logical, consistent and useful
way to tackle it.

_After_ we have made that decision, we can check if it is consistent
with the goal to put it in the next version, and have that version
appear at a reasonable date.  But basing the design on this criterion
in the first place seems dubious to me.

It just appears that a buffer-local replacement list is a kludge for
avoiding a more general scheme of context-dependent faces (probably
related to the XEmacs locale/specifier stuff), and maybe other things.

If the functionality appears very necessary for the release, one can
still decide about a higher level package writer level API for the
problem set to be tackled, and implement this API on top of a cheap
workaround for a single release, after carefully weighing the
benefits.  And then replace workaround and the implementation of the
API in the next release.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Buffer-local faces

[Miles Bader]

On Tue, May 04, 2004 at 11:18:21AM +0200, David Kastrup wrote:
> I am uncomfortable about the whole change.  And the reason has to do
> with the feature freeze.  Now you may argue that the change is not so
> intrusive as to be likely to trigger new bugs, but that's beside the
> point.

I said nothing about the feature freeze.  I did not post my patch to `sneak
in under the wire' of the freeze, I posted it because I (1) happened to have
been working on it, and (2) came up with something nice.

> So I think we should first try to resolve what the perceived problem
> is, what extensions or generalizations of this problem should also be
> solvable, and what would be the most logical, consistent and useful
> way to tackle it.

The patch is in reaction to various past mailing list dicussions; usually
the specific functionality requested is `how can I have a buffer-specific
default/mode-line face' (it always seems to be those two faces).

Anyway, that's what _I_ want to do with it.

> It just appears that a buffer-local replacement list is a kludge for
> avoiding a more general scheme of context-dependent faces (probably
> related to the XEmacs locale/specifier stuff), and maybe other things.

I don't think it's a kludge at all, it's an elegant way of leveraging emacs'
very flexible variable mechanism to achieve the goal -- not only is it almost
trivial to implement, but it would seem to fit very well with the way emacs
modes work.

Perhaps xemacs has a better way of doing it, I don't know -- I'm afraid I'm
not familiar with xemacs in recent years.

-Miles
-- 
((lambda (x) (list x x)) (lambda (x) (list x x)))

Re: Buffer-local faces

[Juri Linkov]

David Kastrup <dak@gnu.org> writes:
> So I think we should first try to resolve what the perceived problem
> is, what extensions or generalizations of this problem should also be
> solvable, and what would be the most logical, consistent and useful
> way to tackle it.

It would be good to generalize it with another useful feature Miles
implemented that allows to specify a filter function applied on faces.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Buffer-local faces

[Stefan Monnier]

> A related questions is whether the remapping should be visible to lisp code
> using face functions.  For instance, does (face-attribute 'default :family)
> 1. return the remapped family, or 2. return the family of the `underlying'
> default face?

The advantage of not provding MERGE-FACE(s) but relying on :inherit instead
is that those problems have already been addressed there.


        Stefan

Re: Buffer-local faces

[Stefan Monnier]

> It's a much less obvious operation (what does subtracting red from black
> yield?  [Ok, maybe cyan :-]) and I can't offhand think of any obvious uses
> for it....

As I suggested, we might want to add a notion of color composition, so we
could have a face that make the foreground darker, or redder, or ...
But supposedly withing RGB this doesn't work so well, so maybe it would be
better to do it on CYK colors.  In any case this is for post-release.

> A _different_ feature that might work is a way to mark a face attribute as
> `non overridable'.  For instance if a face attribute could be a list like
> (fixed VALUE), where VALUE is a normal value for that attribute, face
> merging would always use the `base' value for that attribute instead of the
> `normally overriding' value.

A related question is how to make a face that uses explicitly the default
font.  E.g. let's say my header-line face uses helvetica font and I want to
put text in the header-line that uses the same font as the default font.
Right now, the best I could come up with is to extract the font of `default'
and pass it to `myface', but that's done "statically".

I'm not sure what's a useful generalization, maybe a "filtered inheritance"
where you can specify which attributes are inherited from a given parent.


        Stefan

Re: Buffer-local faces

[Miles Bader]

On Tue, May 04, 2004 at 09:37:52AM -0400, Stefan Monnier wrote:
> > It's a much less obvious operation (what does subtracting red from black
> > yield?  [Ok, maybe cyan :-]) and I can't offhand think of any obvious uses
> > for it....
> 
> As I suggested, we might want to add a notion of color composition, so we
> could have a face that make the foreground darker, or redder, or ...

Yeah, that's one of the areas I've wanted to work on -- I was thinking about
things like `darker'/`lighter'/ `more-intense'/`less-intense' color operators
(the latter two automatically choosing darker or lighter depending on the
context), plus `shaded' versions of the same operators (e.g., doing the
operation with a particular tint).

The reason is so that many faces could be written to be independent of
particular background colors; currently many faces only work well in a single
context (as evidenced by the occasional bickering over face colors :-) --
with such operators, they could even work well in dynamic contexts thanks to
face merging.

I don't know about CMY vs. RGB though -- in my somewhat limited experience,
_all_ color systems suck for this of thing... :-)

> I'm not sure what's a useful generalization, maybe a "filtered inheritance"
> where you can specify which attributes are inherited from a given parent.

Yeah, the question is what's a good way to specify it?  It's always tempting
to just throw in lisp code, but we've seen the messes that gets us into with
the signal handlers &c...

-Miles
-- 
`There are more things in heaven and earth, Horatio,
 Than are dreamt of in your philosophy.'

Re: Buffer-local faces

[Stefan Monnier]

>> I'm not sure what's a useful generalization, maybe a "filtered inheritance"
>> where you can specify which attributes are inherited from a given parent.
> Yeah, the question is what's a good way to specify it?  It's always tempting
> to just throw in lisp code, but we've seen the messes that gets us into with
> the signal handlers &c...

Try -DSYNC_INPUT ;-)
Seriously, tho, the problem is not signal handlers (those are fixed
by -DSYNC_INPUT or by BLOCK_INPUT), but execution of Lisp code from within
the display engine (which can lead to crashes if the Lisp code modifies the
current buffer or does other such funny things).


        Stefan

Re: Buffer-local faces

[Richard Stallman]

    A related questions is whether the remapping should be visible to lisp code
    using face functions.  For instance, does (face-attribute 'default :family)
    1. return the remapped family, or 2. return the family of the `underlying'
    default face?

I think that functions that operate on face names should return the
information about the underlying face, not remapped.  If it is useful,
we could add new functions to get information about a character's
appearance, and they would follow the remapping.

Re: Buffer-local faces

[Richard Stallman]

    Then subtracting face attributes would be useful too.

I think this is much added complexity without much added benefit.
If there are specific attributes you'd like to override,
you can do that by merging a face which has different values
for those attributes.  That does more or less the same job
without the added complexity.

Re: Buffer-local faces

[Richard Stallman]

    A _different_ feature that might work is a way to mark a face attribute as
    `non overridable'.  For instance if a face attribute could be a list like
    (fixed VALUE), where VALUE is a normal value for that attribute, face
    merging would always use the `base' value for that attribute instead of the
    `normally overriding' value.

I really dislike this idea.  Next someone would want a way to mark a
face attribute as super-overriding so as to override the 'non
overridable' marker.  Then someone would want a way to mark a face
attribute as 'really non-overridable'.  I call this the inheritance
arms race.

I see no reason to take even one step down the path.
It isn't really necessary.

Re: Buffer-local faces

[Richard Stallman]

    It would be good to generalize it with another useful feature Miles
    implemented that allows to specify a filter function applied on faces.

That would be going overboard.  This is a simple and clean feature.
I don't think any more is needed.

Re: Buffer-local faces

[Richard Stallman]

    A related question is how to make a face that uses explicitly the default
    font.  E.g. let's say my header-line face uses helvetica font and I want to
    put text in the header-line that uses the same font as the default font.
    Right now, the best I could come up with is to extract the font of `default'
    and pass it to `myface', but that's done "statically".

Complications like these are expensive in terms of the ease of
understanding Emacs.  If we can do without them, let's do without
them.

Re: Buffer-local faces

[Richard Stallman]

    Yeah, that's one of the areas I've wanted to work on -- I was thinking about
    things like `darker'/`lighter'/ `more-intense'/`less-intense' color operators

These could be clean if we make them face attribute values, so that
they participate in merging.

Re: Buffer-local faces

[Richard Stallman]

    Seriously, tho, the problem is not signal handlers (those are fixed
    by -DSYNC_INPUT or by BLOCK_INPUT), but execution of Lisp code from within
    the display engine (which can lead to crashes if the Lisp code modifies the
    current buffer or does other such funny things).

Execution of Lisp code within display is legitimate nowadays.
So if a specific case causes trouble, we need to debug it.

Re: Buffer-local faces

[Stefan Monnier]

>     A related question is how to make a face that uses explicitly the
>     default font.  E.g. let's say my header-line face uses helvetica font
>     and I want to put text in the header-line that uses the same font as
>     the default font.  Right now, the best I could come up with is to
>     extract the font of `default' and pass it to `myface', but that's done
>     "statically".

> Complications like these are expensive in terms of the ease of
> understanding Emacs.  If we can do without them, let's do without them.

Agreed.  I'm looking for a simple and generic solution.
Haven't found it yet,


        Stefan

Re: Buffer-local faces

[Miles Bader]

Richard Stallman <rms@gnu.org> writes:
>     Yeah, that's one of the areas I've wanted to work on -- I was thinking about
>     things like `darker'/`lighter'/ `more-intense'/`less-intense' color operators
> 
> These could be clean if we make them face attribute values, so that
> they participate in merging.

Right, that was the intention.

[And my other secret plan: they make sense for images too...]

-Miles
-- 
In New York, most people don't have cars, so if you want to kill a person, you
have to take the subway to their house.  And sometimes on the way, the train
is delayed and you get impatient, so you have to kill someone on the subway.
  [George Carlin]