face-remapping patch

face-remapping patch

[Miles Bader]

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

I've attached my face-remapping patch to this message (I needed to
untangle it from some other changes).

I've used regularly in my personal emacs for many years, and in light
testing the patch seems to work fine against the current trunk.

-Miles

-- 
Do not taunt Happy Fun Ball.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: face-remapping-20080527-1.patch --]
[-- Type: text/x-diff; name=face-remapping-20080527-1.patch, Size: 23884 bytes --]

orig = emacs@sv.gnu.org/emacs--devo--0--patch-1180
mod = emacs@sv.gnu.org/emacs--devo--0--patch-1180

A  src/ChangeLog.face-remap
M  src/xfaces.c
M  src/dispextern.h
M  src/fontset.c
M  src/xdisp.c
M  doc/lispref/display.texi

--- orig/doc/lispref/display.texi
+++ mod/doc/lispref/display.texi
@@ -2365,6 +2365,58 @@
   When multiple overlays cover one character, an overlay with higher
 priority overrides those with lower priority.  @xref{Overlays}.
 
+@defvar face-remapping-alist
+@tindex face-remapping-alist
+  This variable is used for buffer-local changes in the appearance of
+a face, for instance making the @code{default} face a variable-pitch
+face in a particular buffer.
+
+  Its value should be an alist, whose elements have the form
+@code{(@var{face} @var{remapping}@dots{})}; when the text specifies
+face @var{face}, Emacs redisplay uses @var{remapping}@dots{} instead.
+@var{remapping}@dots{} may be any face specification suitable for a
+@code{face} text property, usually a face name, but also perhaps a
+property list of face attribute/value pairs; @xref{Special
+Properties}.
+
+Two points bear emphasizing:
+
+@enumerate
+@item
+The new definition @var{remapping}@dots{} is the complete
+specification of how to display @var{face}---it entirely replaces,
+rather than augmenting or modifying, the normal definition of that
+face.
+
+@item
+If @var{remapping}@dots{} recursively references the same face name
+@var{face}, either directly remapping entry, or via the
+@code{:inherit} attribute of some other face in
+@var{remapping}@dots{}, then that reference uses normal frame-wide
+definition of @var{face} instead of the `remapped' definition.
+
+For instance, if the @code{mode-line} face is remapped using this
+entry in @code{face-remapping-alist}:
+@example
+(mode-line italic mode-line)
+@end example
+Then the new definition of the @code{mode-line} face inherits from the
+@code{italic} face, and the @emph{normal} (non-remapped) definition of
+@code{mode-line} face.
+@end enumerate
+
+  A typical use of the @code{face-remapping-alist} is to change a
+buffer's @code{default} face; for example, the following changes a
+buffer's @code{default} face to use the @code{variable-pitch} face,
+with the height doubled:
+
+@example
+(set (make-local-variable 'face-remapping-alist)
+     '((default variable-pitch :height 2.0)))
+@end example
+
+@end defvar
+
 @node Font Selection
 @subsection Font Selection
 


--- orig/src/dispextern.h
+++ mod/src/dispextern.h
@@ -2852,6 +2852,7 @@
 int lookup_face P_ ((struct frame *, Lisp_Object *));
 int lookup_non_ascii_face P_ ((struct frame *, 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));
@@ -2880,6 +2881,8 @@
 extern Lisp_Object split_font_name_into_vector P_ ((Lisp_Object));
 extern Lisp_Object build_font_name_from_vector P_ ((Lisp_Object));
 
+extern Lisp_Object Vface_remapping_alist;
+
 /* Defined in xfns.c  */
 
 #ifdef HAVE_X_WINDOWS


--- orig/src/fontset.c
+++ mod/src/fontset.c
@@ -1727,7 +1727,7 @@
       CHECK_CHARACTER (ch);
       c = XINT (ch);
       f = XFRAME (selected_frame);
-      face_id = DEFAULT_FACE_ID;
+      face_id = lookup_basic_face (f, DEFAULT_FACE_ID);
       pos = -1;
       cs_id = -1;
     }


--- orig/src/xdisp.c
+++ mod/src/xdisp.c
@@ -2493,6 +2493,7 @@
      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);
@@ -2509,6 +2510,10 @@
       free_all_realized_faces (Qnil);
     }
 
+  /* Perhaps remap BASE_FACE_ID to a user-specified alternative.  */
+  if (! NILP (Vface_remapping_alist))
+    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)
@@ -2524,7 +2529,7 @@
   bzero (it, sizeof *it);
   it->current.overlay_string_index = -1;
   it->current.dpvec_index = -1;
-  it->base_face_id = base_face_id;
+  it->base_face_id = remapped_base_face_id;
   it->string = Qnil;
   IT_STRING_CHARPOS (*it) = IT_STRING_BYTEPOS (*it) = -1;
 
@@ -2709,11 +2714,11 @@
     {
       struct face *face;
 
-      it->face_id = base_face_id;
+      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, base_face_id);
+      face = FACE_FROM_ID (it->f, remapped_base_face_id);
       if (face->box != FACE_NO_BOX)
 	it->start_of_box_run_p = 1;
     }
@@ -4079,7 +4084,8 @@
 	      /* Value is a multiple of the canonical char height.  */
 	      struct face *face;
 
-	      face = FACE_FROM_ID (it->f, DEFAULT_FACE_ID);
+	      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]));
 	    }
@@ -4189,7 +4195,7 @@
 	  || EQ (XCAR (spec), Qright_fringe))
       && CONSP (XCDR (spec)))
     {
-      int face_id = DEFAULT_FACE_ID;
+      int face_id = lookup_basic_face (it->f, DEFAULT_FACE_ID);
       int fringe_bitmap;
 
       if (!FRAME_WINDOW_P (it->f))


--- orig/src/xfaces.c
+++ mod/src/xfaces.c
@@ -422,6 +422,23 @@
 
 Lisp_Object Vface_new_frame_defaults;
 
+/* Alist of face remappings.  Each element is of the form:
+   (FACE REPLACEMENT...) which causes display of the face FACE to use
+   REPLACEMENT... instead.  REPLACEMENT... is interpreted the same way
+   the value of a `face' text property is: it may be (1) A face name,
+   (2) A list of face names, (3) A property-list of face attribute/value
+   pairs, or (4) A list of face names intermixed with lists containing
+   face attribute/value pairs.
+
+   Multiple entries in REPLACEMENT... are merged together to form the final
+   result, with faces or attributes earlier in the list taking precedence
+   over those that are later.
+
+   Face-name remapping cycles are suppressed; recursive references use
+   the underlying face instead of the remapped face.  */
+
+Lisp_Object Vface_remapping_alist;
+
 /* The next ID to assign to Lisp faces.  */
 
 static int next_lface_id;
@@ -493,7 +510,8 @@
 static Lisp_Object resolve_face_name P_ ((Lisp_Object, int));
 static int may_use_scalable_font_p P_ ((const char *));
 static void set_font_frame_param P_ ((Lisp_Object, Lisp_Object));
-static int get_lface_attributes P_ ((struct frame *, Lisp_Object, Lisp_Object *, int));
+static int get_lface_attributes P_ ((struct frame *, Lisp_Object, Lisp_Object *,
+				     int, struct named_merge_point *));
 static int load_pixmap P_ ((struct frame *, Lisp_Object, unsigned *, unsigned *));
 static unsigned char *xstrlwr P_ ((unsigned char *));
 static struct frame *frame_or_selected_frame P_ ((Lisp_Object, int));
@@ -2058,6 +2076,12 @@
 \f
 /* Face-merge cycle checking.  */
 
+enum named_merge_point_kind
+{
+  NAMED_MERGE_POINT_NORMAL,
+  NAMED_MERGE_POINT_REMAP
+};
+
 /* A `named merge point' is simply a point during face-merging where we
    look up a face by name.  We keep a stack of which named lookups we're
    currently processing so that we can easily detect cycles, using a
@@ -2067,27 +2091,40 @@
 struct named_merge_point
 {
   Lisp_Object face_name;
+  enum named_merge_point_kind named_merge_point_kind;
   struct named_merge_point *prev;
 };
 
 
 /* If a face merging cycle is detected for FACE_NAME, return 0,
    otherwise add NEW_NAMED_MERGE_POINT, which is initialized using
-   FACE_NAME, as the head of the linked list pointed to by
-   NAMED_MERGE_POINTS, and return 1.  */
+   FACE_NAME and NAMED_MERGE_POINT_KIND, as the head of the linked list
+   pointed to by NAMED_MERGE_POINTS, and return 1.  */
 
 static INLINE int
 push_named_merge_point (struct named_merge_point *new_named_merge_point,
 			Lisp_Object face_name,
+			enum named_merge_point_kind named_merge_point_kind,
 			struct named_merge_point **named_merge_points)
 {
   struct named_merge_point *prev;
 
   for (prev = *named_merge_points; prev; prev = prev->prev)
     if (EQ (face_name, prev->face_name))
-      return 0;
+      {
+	if (prev->named_merge_point_kind == named_merge_point_kind)
+	  /* A cycle, so fail.  */
+	  return 0;
+	else if (prev->named_merge_point_kind == NAMED_MERGE_POINT_REMAP)
+	  /* A remap `hides ' any previous normal merge points
+	     (because the remap means that it's actually different face),
+	     so as we know the current merge point must be normal, we
+	     can just assume it's OK.  */
+	  break;
+      }
 
   new_named_merge_point->face_name = face_name;
+  new_named_merge_point->named_merge_point_kind = named_merge_point_kind;
   new_named_merge_point->prev = *named_merge_points;
 
   *named_merge_points = new_named_merge_point;
@@ -2163,24 +2200,19 @@
 
 
 /* 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.  */
-
+   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 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)
+lface_from_face_name_no_resolve (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, signal_p);
-
   if (f)
     lface = assq_no_quit (face_name, f->face_alist);
   else
@@ -2192,9 +2224,28 @@
     signal_error ("Invalid face", face_name);
 
   check_lface (lface);
+
   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;
+{
+  face_name = resolve_face_name (face_name, signal_p);
+  return lface_from_face_name_no_resolve (f, face_name, signal_p);
+}
+
 
 /* Get face attributes of face FACE_NAME from frame-local faces on
    frame F.  Store the resulting attributes in ATTRS which must point
@@ -2203,26 +2254,65 @@
    Otherwise, value is zero if FACE_NAME is not a face.  */
 
 static INLINE int
-get_lface_attributes (f, face_name, attrs, signal_p)
+get_lface_attributes_no_remap (f, face_name, attrs, signal_p)
      struct frame *f;
      Lisp_Object face_name;
      Lisp_Object *attrs;
      int signal_p;
 {
   Lisp_Object lface;
-  int success_p;
 
-  lface = lface_from_face_name (f, face_name, signal_p);
-  if (!NILP (lface))
+  lface = lface_from_face_name_no_resolve (f, face_name, signal_p);
+
+  if (! NILP (lface))
+    bcopy (XVECTOR (lface)->contents, attrs,
+	   LFACE_VECTOR_SIZE * sizeof *attrs);
+
+  return !NILP (lface);
+}
+
+/* Get face attributes of face FACE_NAME from frame-local faces on frame
+   F.  Store the resulting attributes in ATTRS which must point to a
+   vector of Lisp_Objects of size LFACE_VECTOR_SIZE.  If FACE_NAME is an
+   alias for another face, use that face's definition.  If SIGNAL_P is
+   non-zero, signal an error if FACE_NAME does not name a face.
+   Otherwise, value is zero if FACE_NAME is not a face.  */
+
+static INLINE int
+get_lface_attributes (f, face_name, attrs, signal_p, named_merge_points)
+     struct frame *f;
+     Lisp_Object face_name;
+     Lisp_Object *attrs;
+     int signal_p;
+     struct named_merge_point *named_merge_points;
+{
+  Lisp_Object face_remapping;
+
+  face_name = resolve_face_name (face_name, signal_p);
+
+  /* See if SYMBOL has been remapped to some other face (usually this
+     is done buffer-locally).  */
+  face_remapping = assq_no_quit (face_name, Vface_remapping_alist);
+  if (CONSP (face_remapping))
     {
-      bcopy (XVECTOR (lface)->contents, attrs,
-	     LFACE_VECTOR_SIZE * sizeof *attrs);
-      success_p = 1;
+      struct named_merge_point named_merge_point;
+
+      if (push_named_merge_point (&named_merge_point,
+				  face_name, NAMED_MERGE_POINT_REMAP,
+				  &named_merge_points))
+	{
+	  int i;
+
+	  for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
+	    attrs[i] = Qunspecified;
+
+	  return merge_face_ref (f, XCDR (face_remapping), attrs,
+				 signal_p, named_merge_points);
+	}
     }
-  else
-    success_p = 0;
 
-  return success_p;
+  /* Default case, no remapping.  */
+  return get_lface_attributes_no_remap (f, face_name, attrs, signal_p);
 }
 
 
@@ -2378,8 +2468,8 @@
    specified attribute of FROM overrides the corresponding attribute of
    TO; relative attributes in FROM are merged with the absolute value in
    TO and replace it.  NAMED_MERGE_POINTS is used internally to detect
-   loops in face inheritance; it should be 0 when called from other
-   places.  */
+   loops in face inheritance/remapping; it should be 0 when called from
+   other places.  */
 
 static INLINE void
 merge_face_vectors (f, from, to, named_merge_points)
@@ -2454,11 +2544,12 @@
   struct named_merge_point named_merge_point;
 
   if (push_named_merge_point (&named_merge_point,
-			      face_name, &named_merge_points))
+			      face_name, NAMED_MERGE_POINT_NORMAL,
+			      &named_merge_points))
     {
       struct gcpro gcpro1;
       Lisp_Object from[LFACE_VECTOR_SIZE];
-      int ok = get_lface_attributes (f, face_name, from, 0);
+      int ok = get_lface_attributes (f, face_name, from, 0, named_merge_points);
 
       if (ok)
 	{
@@ -3435,8 +3526,8 @@
       Lisp_Object frame;
 
       /* Changing the background color might change the background
-	 mode, so that we have to load new defface specs.
-	 Call frame-set-background-mode to do that.  */
+	 mode, so that we have to load new defface specs.  Call
+	 frame-update-face-colors to do that.  */
       XSETFRAME (frame, f);
       call1 (Qframe_set_background_mode, frame);
 
@@ -4623,8 +4714,8 @@
    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, signal_p)
+static int
+lookup_named_face_1 (f, symbol, signal_p)
      struct frame *f;
      Lisp_Object symbol;
      int signal_p;
@@ -4642,7 +4733,7 @@
 	abort ();  /* realize_basic_faces must have set it up  */
     }
 
-  if (!get_lface_attributes (f, symbol, symbol_attrs, signal_p))
+  if (! get_lface_attributes (f, symbol, symbol_attrs, signal_p, 0))
     return -1;
 
   bcopy (default_face->lface, attrs, sizeof attrs);
@@ -4651,6 +4742,67 @@
   return lookup_face (f, attrs);
 }
 
+/* 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, signal_p)
+     struct frame *f;
+     Lisp_Object symbol;
+     int signal_p;
+{
+  return lookup_named_face_1 (f, symbol, 0);
+}
+
+
+/* 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_remapping_alist.  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_remapping_alist))
+    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;
+    case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
+    case FRINGE_FACE_ID:		name = Qfringe;			break;
+    case SCROLL_BAR_FACE_ID:		name = Qscroll_bar;		break;
+    case BORDER_FACE_ID:		name = Qborder;			break;
+    case CURSOR_FACE_ID:		name = Qcursor;			break;
+    case MOUSE_FACE_ID:			name = Qmouse;			break;
+    case MENU_FACE_ID:			name = Qmenu;			break;
+
+    default:
+      return face_id;		/* Give up.  */
+    }
+
+  mapping = assq_no_quit (name, Vface_remapping_alist);
+  if (NILP (mapping))
+    return face_id;		/* Give up.  */
+
+  remapped_face_id = lookup_named_face_1 (f, name, 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.  */
@@ -4784,7 +4936,7 @@
   if (!default_face)
     abort ();
 
-  get_lface_attributes (f, symbol, symbol_attrs, signal_p);
+  get_lface_attributes (f, symbol, symbol_attrs, signal_p, 0);
   bcopy (default_face->lface, attrs, sizeof attrs);
   merge_face_vectors (f, symbol_attrs, attrs, 0);
   return lookup_face (f, attrs);
@@ -4810,7 +4962,6 @@
 			Face capability testing
  ***********************************************************************/
 
-
 /* If the distance (as returned by color_distance) between two colors is
    less than this, then they are considered the same, for determining
    whether a color is supported or not.  The range of values is 0-65535.  */
@@ -4943,7 +5094,6 @@
   unsigned test_caps = 0;
   Lisp_Object *def_attrs = def_face->lface;
 
-
   /* First check some easy-to-check stuff; ttys support none of the
      following attributes, so we can just return false if any are requested
      (even if `nominal' values are specified, we should still return false,
@@ -5493,7 +5643,7 @@
   struct face *new_face;
 
   /* The default face must exist and be fully specified.  */
-  get_lface_attributes (f, Qdefault, attrs, 1);
+  get_lface_attributes_no_remap (f, Qdefault, attrs, 1);
   check_lface_attrs (attrs);
   xassert (lface_fully_specified_p (attrs));
 
@@ -5506,7 +5656,7 @@
     }
 
   /* Merge SYMBOL's face with the default face.  */
-  get_lface_attributes (f, symbol, symbol_attrs, 1);
+  get_lface_attributes_no_remap (f, symbol, symbol_attrs, 1);
   merge_face_vectors (f, symbol_attrs, attrs, 0);
 
   /* Realize the face.  */
@@ -6063,13 +6213,18 @@
 
   *endptr = endpos;
 
-  default_face = FACE_FROM_ID (f, DEFAULT_FACE_ID);
+
+  /* Perhaps remap BASE_FACE_ID to a user-specified alternative.  */
+  if (NILP (Vface_remapping_alist))
+    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;
+    return default_face->id;
 
   /* Begin with attributes from the default face.  */
   bcopy (default_face->lface, attrs, sizeof attrs);
@@ -6668,6 +6823,43 @@
 ignore.  */);
   Vface_ignored_fonts = Qnil;
 
+  DEFVAR_LISP ("face-remapping-alist", &Vface_remapping_alist,
+	       doc: /* Alist of face remappings.
+Each element is of the form:
+
+   (FACE REPLACEMENT...),
+
+which causes display of the face FACE to use REPLACEMENT... instead.
+REPLACEMENT... is interpreted the same way the value of a `face' text
+property is: it may be (1) A face name, (2) A list of face names, (3) A
+property-list of face attribute/value pairs, or (4) A list of face names
+intermixed with lists containing face attribute/value pairs.
+
+Multiple entries in REPLACEMENT... are merged together to form the final
+result, with faces or attributes earlier in the list taking precedence
+over those that are later.
+
+Face-name remapping cycles are suppressed; recursive references use the
+underlying face instead of the remapped face.  So a remapping of the form:
+
+   (FACE EXTRA-FACE... FACE)
+
+or:
+
+   (FACE (FACE-ATTR VAL ...) FACE)
+
+will cause EXTRA-FACE... or (FACE-ATTR VAL ...) to be _merged_ with the
+existing definition of FACE.  Note that for the default face, this isn't
+necessary, as every face inherits from the default face.
+
+Making this variable buffer-local is a good way to allow buffer-specific
+face definitions.  For instance, the mode my-mode could define a face
+`my-mode-default', and then in the mode setup function, do:
+
+   (set (make-local-variable 'face-remapping-alist)
+        '((default my-mode-default)))).  */);
+  Vface_remapping_alist = 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



* added files

--- /dev/null
+++ mod/src/ChangeLog.face-remap
@@ -0,0 +1,43 @@
+2005-05-03  Miles Bader  <miles@gnu.org>
+
+	* xfaces.c (get_lface_attributes): Pass SIGNAL_P to resolve_face_name.
+
+2004-05-25  Miles Bader  <miles@gnu.org>
+
+	* xfaces.c (Vface_remapping_alist): New variable.
+	(syms_of_xfaces): Initialize it.
+	(enum named_merge_point_kind): New type.
+	(struct named_merge_point): Add `named_merge_point_kind' field.
+	(push_named_merge_point): Make cycle detection respect different
+	named-merge-point kinds.
+	(lface_from_face_name_no_resolve): Renamed from `lface_from_face_name'.
+	Remove face-name alias resolution.
+	(lface_from_face_name): New definition using
+	`lface_from_face_name_no_resolve'.
+	(get_lface_attributes_no_remap): Renamed from `get_lface_attributes'.
+	Call lface_from_face_name_no_resolve instead of lface_from_face_name.
+	(get_lface_attributes): New definition that layers face-remapping on
+	top of get_lface_attributes_no_remap.  New arg `named_merge_points'.
+	(lookup_named_face_1): Renamed from `lookup_named_face'.  Add
+	`signal_p' argument.  Pass new last arg to `get_lface_attributes', and
+	return -1 if it fails.
+	(lookup_named_face): Redefined in terms of `lookup_named_face_1'.
+	(lookup_basic_face): New function.
+	(lookup_derived_face): Pass new last arg to `get_lface_attributes'.
+	(realize_named_face): Call `get_lface_attributes_no_remap' instead of
+	`get_lface_attributes'.
+	(face_at_buffer_position): Use `lookup_basic_face' to lookup
+	DEFAULT_FACE_ID if necessary.  When optimizing the default-face case,
+	return default_face's face-id instead of the constant DEFAULT_FACE_ID.
+
+	* xdisp.c (init_iterator): Pass base_face_id through
+	`lookup_basic_face' when we actually use it as a face-id.
+	(handle_single_display_prop): Use `lookup_basic_face' to lookup
+	DEFAULT_FACE_ID.
+
+	* fontset.c (Finternal_char_font): Use `lookup_basic_face' to
+	lookup the initial face-id.
+
+	* dispextern.h (lookup_basic_face, Vface_remapping_alist): New decls.
+
+;; arch-tag: aded4c61-a649-4b57-9c93-5408933a7b40

Re: face-remapping patch

[Florian Beck]

"Miles Bader" <miles@gnu.org> writes:

> I've attached my face-remapping patch to this message (I needed to
> untangle it from some other changes).

Hi,

this seems to work very nicely so far. 

Two questions: Firstly, this works strictly on faces, doesn't it? I
cannot, say, have kanji – or any non-latin characters - displayed at
double height.

Secondly, what I'd really like is a way to change fonts/faces not per
buffer but per window. (Use cases: a secondary window which displays notes
in a smaller font or which displays tabular data in monospace font.) I
hoped I could set `face-remapping-alist'  when a buffer is displayed in a
certain window and clear it otherwise, but I am unable to find the right
hook for that.

Thanks anyway
-- 
Florian Beck

Re: face-remapping patch

[Miles Bader]

Florian Beck <abstraktion@t-online.de> writes:
> Two questions: Firstly, this works strictly on faces, doesn't it?

Yes

> Secondly, what I'd really like is a way to change fonts/faces not per
> buffer but per window. (Use cases: a secondary window which displays notes
> in a smaller font or which displays tabular data in monospace font.)

If there's demand for such a feature, maybe a per-window remapping list
could be added as well (e.g., using `window-face-remappings' and
`set-window-face-remappings' functions), whose entries would take
precedence over the normal variable.

-Miles

-- 
Future, n. That period of time in which our affairs prosper, our friends
are true and our happiness is assured.

Re: face-remapping patch

[Richard M Stallman]

    Secondly, what I'd really like is a way to change fonts/faces not per
    buffer but per window.

I think that will only be useful once there are features to make
certain windows exist in a more permanent way.  (People are working on
those.)

Re: face-remapping patch

[Stefan Monnier]

> I've attached my face-remapping patch to this message (I needed to
> untangle it from some other changes).

> I've used regularly in my personal emacs for many years, and in light
> testing the patch seems to work fine against the current trunk.

It looks pretty good.  Is there any reason why you added redundant tests
like !NILP (Vface_remapping_alist)?  I'd expect at least in my case that
face-remapping-alist would almost never be nil so this extra test would
end up redundant.


        Stefan


> +int
> +lookup_named_face (f, symbol, signal_p)
> +     struct frame *f;
> +     Lisp_Object symbol;
> +     int signal_p;
> +{
> +  return lookup_named_face_1 (f, symbol, 0);
> +}

Why have a `signal_p' arg that's not used?
Also, the ChangeLog entry doesn't make much sense:

	(lookup_named_face_1): Renamed from `lookup_named_face'.  Add
	`signal_p' argument.  Pass new last arg to `get_lface_attributes', and
	return -1 if it fails.
	(lookup_named_face): Redefined in terms of `lookup_named_face_1'.

The signal_p arg was there before, it's not added.  Am I missing something?

> +  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;
> +    case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
> +    case FRINGE_FACE_ID:		name = Qfringe;			break;
> +    case SCROLL_BAR_FACE_ID:		name = Qscroll_bar;		break;
> +    case BORDER_FACE_ID:		name = Qborder;			break;
> +    case CURSOR_FACE_ID:		name = Qcursor;			break;
> +    case MOUSE_FACE_ID:			name = Qmouse;			break;
> +    case MENU_FACE_ID:			name = Qmenu;			break;
> +
> +    default:
> +      return face_id;		/* Give up.  */

Does this default case correspond to a bug?  if so shouldn't it `abort'?
If not, shouldn't it obey Vface_remapping_alist?

> +  mapping = assq_no_quit (name, Vface_remapping_alist);
> +  if (NILP (mapping))
> +    return face_id;		/* Give up.  */
> +
> +  remapped_face_id = lookup_named_face_1 (f, name, 0);

This looks odd: we look up Vface_remapping_alist and then ignore the
actual result (other than its being null or not).  Is it just an
optimization to avoid calling lookup_named_face_1?  If so, a comment is
in order.

Re: face-remapping patch

[Miles Bader]

On Wed, May 28, 2008 at 11:54 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> Is there any reason why you added redundant tests
> like !NILP (Vface_remapping_alist)?  I'd expect at least in my case that
> face-remapping-alist would almost never be nil so this extra test would
> end up redundant.

There's only one such test.   :-)

Anyway, the default value is nil, and while modes and users might
start using it, still, I think it would be nil very commonly.

> Why have a `signal_p' arg that's not used?
> Also, the ChangeLog entry doesn't make much sense:
>
>        (lookup_named_face_1): Renamed from `lookup_named_face'.  Add
>        `signal_p' argument.  Pass new last arg to `get_lface_attributes', and
>        return -1 if it fails.
>        (lookup_named_face): Redefined in terms of `lookup_named_face_1'.
>
> The signal_p arg was there before, it's not added.  Am I missing something?

Can't exactly say what's going on here, but this code is pretty old,
so it's possible the trunk code has been changed since the ChangeLog
entry was written.  Indeed I remember being annoyed at the conflicts
that arose when a bunch of that code got re-arranged at some point....
 :-)

>> +  switch (face_id)
>> +    {
>> +    case DEFAULT_FACE_ID:            name = Qdefault;                break;
...
>> +    default:
>> +      return face_id;                /* Give up.  */
>
> Does this default case correspond to a bug?  if so shouldn't it `abort'?
> If not, shouldn't it obey Vface_remapping_alist?

Yeah it's a bug; callers to that function are only supposed to pass
one of the "basic" face ids.

>> +  mapping = assq_no_quit (name, Vface_remapping_alist);
>> +  if (NILP (mapping))
>> +    return face_id;          /* Give up.  */
>> +
>> +  remapped_face_id = lookup_named_face_1 (f, name, 0);
>
> This looks odd: we look up Vface_remapping_alist and then ignore the
> actual result (other than its being null or not).  Is it just an
> optimization to avoid calling lookup_named_face_1?  If so, a comment is
> in order.

Insofar as I remember, yeah, just an "optimization".  The assumption
is that 99% of the time, there's no remapping (and for most of those
faces, it's probably true), and I think this function is called a lot.

-Miles

-- 
Do not taunt Happy Fun Ball.

Re: face-remapping patch

[David Reitter]

Miles, could you clarify why you are proposing a remapping list rather  
than "buffer-local faces"?

Buffer-local faces would behave like buffer-local variables, and they  
would also be analogous to faces specific to a frame.
Such functionality would come with basic house-keeping, i.e. garbage  
collection for locally defined faces that aren't needed any longer  
after the buffer has been killed.

Re: face-remapping patch

[Miles Bader]

David Reitter <david.reitter@gmail.com> writes:
> Miles, could you clarify why you are proposing a remapping list rather
> than "buffer-local faces"?

How would those work, exactly?

[E.g., what would your proposed interface be?]

-Miles

-- 
My books focus on timeless truths.  -- Donald Knuth

Re: face-remapping patch

[Miles Bader]

BTW, one reason for why a remapping alist is so nice is that it
leverages Emacs' existing infrastructure well.  In particular, the
user-related parts like face customization are built around global named
faces (even frame-specific faces are not handled well [at all?] by
this).

When a major mode to remaps `default' to `my-mode-default', existing
mechanisms can be used to handle things like user-customization of
`my-mode-default'.

-Miles

-- 
Virtues, n. pl. Certain abstentions.

Re: face-remapping patch

[David Kastrup]

Miles Bader <miles.bader@necel.com> writes:

> BTW, one reason for why a remapping alist is so nice is that it
> leverages Emacs' existing infrastructure well.  In particular, the
> user-related parts like face customization are built around global
> named faces (even frame-specific faces are not handled well [at all?]
> by this).
>
> When a major mode to remaps `default' to `my-mode-default', existing
> mechanisms can be used to handle things like user-customization of
> `my-mode-default'.

This really sounds like a problem space usually addressed with
"specifiers" in XEmacs.  Unfortunately, I have never been able to figure
them out.

-- 
David Kastrup

Re: face-remapping patch

[Miles Bader]

David Kastrup <dak@gnu.org> writes:
>> When a major mode to remaps `default' to `my-mode-default', existing
>> mechanisms can be used to handle things like user-customization of
>> `my-mode-default'.
>
> This really sounds like a problem space usually addressed with
> "specifiers" in XEmacs.  Unfortunately, I have never been able to figure
> them out.

That doesn't make them sound very promising...

-Miles

-- 
Hers, pron. His.

Re: face-remapping patch

[David Kastrup]

Miles Bader <miles@gnu.org> writes:

> David Kastrup <dak@gnu.org> writes:
>>> When a major mode to remaps `default' to `my-mode-default', existing
>>> mechanisms can be used to handle things like user-customization of
>>> `my-mode-default'.
>>
>> This really sounds like a problem space usually addressed with
>> "specifiers" in XEmacs.  Unfortunately, I have never been able to figure
>> them out.
>
> That doesn't make them sound very promising...

As far as I can tell, XEmacs uses them to map pretty much everything
(images, toolbars, possibly also keymaps, faces) to "locales", namely
buffers, windows, displays.  So you can have images that get
instantiated differently depending on the output device.  The
instantiation works with some sort of caching mechanism and is pretty
efficient.

That's my basic handwaving about the functionality as I think I have
been able to understand.  The implementation is, as far as I can tell,
not done using opaque data structures but rather some sort of alist-like
stuff.  This would actually be more like an Emacs-typical approach to
data structures rather than an XEmacs approach.

The problem when I tried figuring them out is that there was not an
Emacs quality approach to either the documentation or the APIs.

A unified approach to "terminal-, buffer-, window-, frame-local"
certainly would have quite a bit of appeal.  Even if XEmacs specifiers
are good for all of that (I am not sure about it, actually), they don't
unify with normal buffer-locality but have a separate mechanims.  They
also don't work transparently like buffer-locality: dealing with
specifier-locality needs to be done manually, and there is not even a
feature-complete API for all operations you would need.

But that does not mean that taking a thorough look at the problem space
presumably covered by XEmacs specifiers would be amiss.

-- 
David Kastrup

Re: face-remapping patch

[David Reitter]

On 28 May 2008, at 08:29, Miles Bader wrote:

> David Reitter <david.reitter@gmail.com> writes:
>> Miles, could you clarify why you are proposing a remapping list  
>> rather
>> than "buffer-local faces"?
>
> How would those work, exactly?
>
> [E.g., what would your proposed interface be?]


Okay, not to criticize your remapping, but just for the sake of  
argument, here's a possible alternative:

A face name refers to the buffer-local face if defined in (current- 
buffer), and to the global one otherwise.

;; causes any modifications to the face face
;; to result in a buffer-local copy of the face
(make-face-buffer-local face)

;; makes a buffer-local copy of the global face face
;; (or defines a new buffer-local one that inherits from the global one)
;; and modifies that
(modify-face (make-local-face face) ...)

;; non-nil if face is local in buffer buffer.
(local-face-p face &optional buffer)

;;Make face no longer have a separate definition in the current buffer.
(kill-local-face face)

When a buffer is killed, so are all its buffer-local faces.

> BTW, one reason for why a remapping alist is so nice is that it
> leverages Emacs' existing infrastructure well.  In particular, the
> user-related parts like face customization are built around global  
> named
> faces (even frame-specific faces are not handled well [at all?] by
> this).


My experience with this infrastructure, using `color-themes' and  
plenty of frames, has been less than stellar.  When switching color  
themes, more and more faces get defined during the session. This slows  
the creation of frames down to the point where it takes 2+ seconds to  
create one.  I had to install a patch in my own version to keep it  
from making frame-specific copies of all global faces for each new  
frame.

Would buffer-local faces lead to better house-keeping? They could be  
thrown away easily with the buffer and reduce the need for global  
faces. 

Re: face-remapping patch

[Miles Bader]

David Reitter <david.reitter@gmail.com> writes:
> Okay, not to criticize your remapping, but just for the sake of
> argument, here's a possible alternative:

How would user customization work?

>> BTW, one reason for why a remapping alist is so nice is that it
>> leverages Emacs' existing infrastructure well.  In particular, the
>> user-related parts like face customization are built around global
>> named faces (even frame-specific faces are not handled well [at all?]
>> by this).
>
> My experience with this infrastructure, using `color-themes' and plenty
> of frames, has been less than stellar.

Well the implementation could certainly be improved, but adding support
for non-global faces certainly won't simplify things!

Making faces more light-weight would be a good thing I think.

[I won't speak to color-themes; afaik, it's a complete disaster.]

-Miles

-- 
Road, n. A strip of land along which one may pass from where it is too
tiresome to be to where it is futile to go.

Re: face-remapping patch

[David Reitter]

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

On 28 May 2008, at 14:21, Miles Bader wrote:

> David Reitter <david.reitter@gmail.com> writes:
>> Okay, not to criticize your remapping, but just for the sake of
>> argument, here's a possible alternative:
>
> How would user customization work?

I was looking at this from a programmer's point of view (who would  
provide a package).
 From a user's perspective, the local faces could be defined just like  
local variables are set within files.
Ideally there would be a function to add all local faces (as a  
comment) to the end of a buffer.
There should be a function called `customize-local-face'.

> Well the implementation could certainly be improved, but adding  
> support
> for non-global faces certainly won't simplify things!

Certainly not for Emacs, but for anyone using the API that you're  
creating, buffer-local faces would be simple to understand and more  
transparent than another extra variable that remaps some faces.
What would the user customization look like in that case?
E.g. when a user does M-x customize-face default, would they be  
customizing the remapped (local) face or the global one?

- D

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

Face realization (was: face-remapping patch)

[Stefan Monnier]

> My experience with this infrastructure, using `color-themes' and
> plenty of frames, has been less than stellar.  When switching color
> themes, more and more faces get defined during the session. This slows
> the creation of frames down to the point where it takes 2+ seconds to
> create one.  I had to install a patch in my own version to keep it
> from making frame-specific copies of all global faces for each
> new  frame.

This is a separate problem (at least, given the current code).  It would
be good to try and realize faces lazily as they get encountered rather
than realize them all everytime we create a new frame.

Also, as you point out, there's a gross inefficiency in that most faces
are really terminal-local rather than frame-local (after all, the
defface spec does not allow per-frame customizations), so we should
share the resulting realized faces.  If you have a patch for it, maybe
you should post it here,


        Stefan

Re: Face realization (was: face-remapping patch)

[David Reitter]

On 28 May 2008, at 20:25, Stefan Monnier wrote:

> Also, as you point out, there's a gross inefficiency in that most  
> faces
> are really terminal-local rather than frame-local (after all, the
> defface spec does not allow per-frame customizations), so we should
> share the resulting realized faces.  If you have a patch for it, maybe
> you should post it here,


Well, my patch is rather small.  I simply don't run `make-face-x- 
resource-internal' for each frame.
I think there's some way to specify the equivalent of X resources on  
Carbon, but that's nothing that justified the need for an operation  
that delayed frame creation by that much.
This has worked for a lot of users since I started using it, and it  
sped up frame creation enormously.

I really don't know anything about X, so apologies if the below trick  
won't work in X based environment.



*** lisp/faces.el	13 Apr 2007 18:24:53 +0100	1.370
--- lisp/faces.el	13 Jul 2007 18:56:06 +0100	
***************
*** 320,326 ****

   (defun set-face-attributes-from-resources (face frame)
     "Set attributes of FACE from X resources for FRAME."
!   (when (memq (framep frame) '(x w32 mac))
       (dolist (definition face-x-resources)
         (let ((attribute (car definition)))
   	(dolist (entry (cdr definition))
--- 320,326 ----

   (defun set-face-attributes-from-resources (face frame)
     "Set attributes of FACE from X resources for FRAME."
!   (when (eq (framep frame) 'x)
       (dolist (definition face-x-resources)
         (let ((attribute (car definition)))
   	(dolist (entry (cdr definition))
***************
*** 1789,1796 ****
         (condition-case ()
   	  (progn
   	    (face-spec-set face (face-user-default-spec face) frame)
! 	    (if (memq window-system '(x w32 mac))
! 		(make-face-x-resource-internal face frame))
   	    (internal-merge-in-global-face face frame))
   	(error nil)))
       ;; Apply the attributes specified by frame parameters.  This
--- 1789,1796 ----
         (condition-case ()
   	  (progn
   	    (face-spec-set face (face-user-default-spec face) frame)
! 	     (if (eq window-system 'x)
!  		(make-face-x-resource-internal face frame))
   	    (internal-merge-in-global-face face frame))
   	(error nil)))
       ;; Apply the attributes specified by frame parameters.  This

Re: Face realization

[Stefan Monnier]

>> Also, as you point out, there's a gross inefficiency in that most faces
>> are really terminal-local rather than frame-local (after all, the
>> defface spec does not allow per-frame customizations), so we should
>> share the resulting realized faces.  If you have a patch for it, maybe
>> you should post it here,

> Well, my patch is rather small.  I simply don't run `make-face-x-
> resource-internal' for each frame.
> I think there's some way to specify the equivalent of X resources on Carbon,
> but that's nothing that justified the need for an operation  that delayed
> frame creation by that much.
> This has worked for a lot of users since I started using it, and it sped up
> frame creation enormously.

> I really don't know anything about X, so apologies if the below trick won't
> work in X based environment.

[ Me, disappointed. ]
Your patch points out a good place where we could improve speed
noticeably, but clearly it can't be applied as is.

OTOH, it's not clear how to speed it up, especially in the case of X11.

Maybe we should simply process face settings via Xresource elsewhere:
not when creating frames, but when evaluating defface (or at startup
for the pre-defined faces, of course).

The current face-setting code (which merges settings from x-resources,
frame-parameters, defface specs, and manual calls to
set-face-attributes) is really messy and inefficient.


        Stefan

Re: face-remapping patch

[Stefan Monnier]

> Miles, could you clarify why you are proposing a remapping list rather than
> "buffer-local faces"?

I think adding "buffer-local faces" directly is no better.  The good
thing about Miles's patch is that it leverages buffer-local variables to
modify faces buffer-locally.  As a result, his patch is pretty small.

To improve on this, I think we'd have to move to something more like
XEmacs's specifiers.  But I haven't even seen any proposal for such
a thing yet.


        Stefan

Re: face-remapping patch

[David Kastrup]

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

>> Miles, could you clarify why you are proposing a remapping list rather than
>> "buffer-local faces"?
>
> I think adding "buffer-local faces" directly is no better.  The good
> thing about Miles's patch is that it leverages buffer-local variables to
> modify faces buffer-locally.  As a result, his patch is pretty small.
>
> To improve on this, I think we'd have to move to something more like
> XEmacs's specifiers.  But I haven't even seen any proposal for such
> a thing yet.

Proposal: allow a third argument for make-local-variable.

(make-local-variable VARIABLE &optional locus)

Make VARIABLE have a separate value in the given locus (defaulting to
the current buffer).

A locus can be a buffer, a frame, a terminal, a window.  When there is
more than one locus for which a variable may be local, the value is used
from the first locus in the list window - buffer - frame - terminal.

(local-variable-p VARIABLE &optional LOCUS)

Non-nil if VARIABLE has a local binding in locus LOCUS (see make-variable).
BUFFER defaults to the current buffer.

variable-binding-locus is a built-in function in `C source code'.

(variable-binding-locus VARIABLE)

Return a value indicating where VARIABLE's current binding comes from.
If the current binding is buffer-local, the value is the current buffer.
If the current binding is frame-local, the value is the selected frame.
If the current binding is global (the default), the value is nil.



Bing.  A simple API, easy to understand, easy to use.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: face-remapping patch

[David Kastrup]

Sorry, C-c C-c is just too close to C-x C-x.  This posting was not
finished.

David Kastrup <dak@gnu.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> Miles, could you clarify why you are proposing a remapping list rather than
>>> "buffer-local faces"?
>>
>> I think adding "buffer-local faces" directly is no better.  The good
>> thing about Miles's patch is that it leverages buffer-local variables to
>> modify faces buffer-locally.  As a result, his patch is pretty small.
>>
>> To improve on this, I think we'd have to move to something more like
>> XEmacs's specifiers.  But I haven't even seen any proposal for such
>> a thing yet.
>
> Proposal: allow a third argument for make-local-variable.
>
> (make-local-variable VARIABLE &optional locus)
>
> Make VARIABLE have a separate value in the given locus (defaulting to
> the current buffer).
>
> A locus can be a buffer, a frame, a terminal, a window.  When there is
> more than one locus for which a variable may be local, the value is used
> from the first locus in the list window - buffer - frame - terminal.
>
> (local-variable-p VARIABLE &optional LOCUS)
>
> Non-nil if VARIABLE has a local binding in locus LOCUS (see make-variable).
> BUFFER defaults to the current buffer.
>
> variable-binding-locus is a built-in function in `C source code'.
>
> (variable-binding-locus VARIABLE)
>
> Return a value indicating where VARIABLE's current binding comes from.
> If the current binding is buffer-local, the value is the current buffer.
> If the current binding is frame-local, the value is the selected frame.
> If the current binding is global (the default), the value is nil.

That one actually already exists.

For the following, s/buffer/locus/g:

    buffer-local-value is a built-in function in `C source code'.

    (buffer-local-value VARIABLE BUFFER)

    Return the value of VARIABLE in BUFFER.
    If VARIABLE does not have a buffer-local binding in BUFFER, the value
    is the default binding of the variable.

Bing.  A simple API, easy to understand, easy to use.  And
_transparent_.

Now XEmacs specifiers offer something more: you can specify local values
for _classes_ of display, like B&W, or reduced color or so.  We have
something like that for faces.

However, I don't think the price for the generality of specifiers is
worth the complexity in the particular implementation and documentation
state of XEmacs.  I did not understand them after trying for a
considerable amount of time, and I am arrogant enough to consider
something which I feel unable to understand as being off the complexity
bell curve for reliable engineering.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: face-remapping patch

[tomas]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, May 28, 2008 at 10:31:46PM +0200, David Kastrup wrote:
> 
> Sorry, C-c C-c is just too close to C-x C-x.  This posting was not
> finished.
> 
> David Kastrup <dak@gnu.org> writes:

[...]

> > (make-local-variable VARIABLE &optional locus)
> >
> > Make VARIABLE have a separate value in the given locus (defaulting to
> > the current buffer).
> >
> > A locus can be a buffer, a frame, a terminal, a window.  When there is
> > more than one locus for which a variable may be local, the value is used
> > from the first locus in the list window - buffer - frame - terminal.
[...]
> Bing.  A simple API, easy to understand, easy to use.  And
> _transparent_.

Appealing. Humble suggestion: make the list of possible loci extensible,
with the above mentioned being the minimal, "built-in" set. I don't know
yet how an interface would look like, but several other classes of loci
suggest themselves: major modes, (human) languages, what not.

> Now XEmacs specifiers offer something more: you can specify local values
> for _classes_ of display, like B&W, or reduced color or so.  We have
> something like that for faces.

Hm. Would that not be covered by your proposal? You'd just need a little
bit more of indirection. Your locus-local variable (locus=terminal) wouldn't
be a face itself, but a display class -- which then determines the face.

Whether it's worth it -- I don't know.

> However, I don't think the price for the generality of specifiers is
> worth the complexity in the particular implementation and documentation
> state of XEmacs.  I did not understand them after trying for a
> considerable amount of time, and I am arrogant enough to consider
> something which I feel unable to understand as being off the complexity
> bell curve for reliable engineering.

:-)

Still I read your posts carefully.

Regards
- -- tomás
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIPkbzBcgs9XrR2kYRAgRCAJ4vLVerFAvcn7CgyS+u4sI98B6AIgCdEu0Y
px8GI/err2ku9Cb7fKpbelQ=
=OxM9
-----END PGP SIGNATURE-----

Re: face-remapping patch

[Stephen J. Turnbull]

David Kastrup writes:

 > However, I don't think the price for the generality of specifiers is
 > worth the complexity in the particular implementation and documentation
 > state of XEmacs.  I did not understand them after trying for a
 > considerable amount of time,

AFAIK David never did review the documentation after it was revised to
address his problem report.  Specifiers really are not that
complicated for most uses.

In practice, it turns out that 

    (specifier-instance SPECIFIER)

tells you what value is used at point in the selected window, and

    (specifier-instance SPECIFIER DOMAIN)

tells you would value would be used in DOMAIN, which may be a window,
frame, or device (terminal in Emacs lingo).  Note that because a
buffer may be visible in several places with different physical
characteristics that affect rendering (a feature that Emacs did not
support before multi-tty, AIUI), DOMAIN cannot be a buffer.

To set a specifier, use

    (set-specifier SPECIFIER VALUE LOCALE)

where LOCALE defaults to 'global, and may be a window, buffer, frame,
or device.  Typically `specifier-instance' does not return the same
VALUE that `set-specifier' sets, for example you can set the
background color of the default face to "gray80" in the current
buffer, but if you then check the value using `specifier-instance' it
will be "white".  Also `specifier-instance' returns face, font, and
glyph objects rather than their names IIRC.

Re: face-remapping patch

[David Kastrup]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

> David Kastrup writes:
>
>  > However, I don't think the price for the generality of specifiers is
>  > worth the complexity in the particular implementation and documentation
>  > state of XEmacs.  I did not understand them after trying for a
>  > considerable amount of time,
>
> AFAIK David never did review the documentation after it was revised to
> address his problem report.  Specifiers really are not that
> complicated for most uses.

preview-latex contains the following in prv-xemacs.el (and yes, this was
quite after the documentation had been "revised" and I wasted several
hours again):

    (when preview-tb-icon
      (setq preview-tb-icon
	    (vector
	     (list preview-tb-icon)
	     #'preview-at-point
	     t
	     "Preview on/off at point"))))
;;; [Courtesy Stephen J. Turnbull, with some modifications
;;;  Message-ID: <87el9fglsj.fsf@tleepslib.sk.tsukuba.ac.jp>
;;;  I could not have figured this out for the world]
;;; Hm, there really ought to be a way to get the spec that would be
;;; instantiated in a given domain
  (when preview-tb-icon
    (let ((tb (cdadar (or (specifier-spec-list default-toolbar (current-buffer))
			  (specifier-spec-list default-toolbar 'global)))))
      (unless (member preview-tb-icon tb)
	(set-specifier default-toolbar
		       (append tb (list preview-tb-icon))
		       (current-buffer)))))

Sorry, but that is just not what I see as an API people with my
stupidity level can be considered to be able to work with.  The
corresponding passage for Emacs reads:

  (when preview-tb-icon
    (define-key LaTeX-mode-map [tool-bar preview]
      `(menu-item "Preview at point" preview-at-point
		  :image ,preview-tb-icon
		  :help "Preview on/off at point")))

That is something that I can actually not only read, but which I could
also write.  Yes, it helped that Emacs documentation was useful, all in
one place and with actual examples.  But even supposing that the XEmacs
documentation might have caught up, the code you have to write is still
a incoherent mess for someone like me.  Documenting the incoherent mess
is only taking me so far forward.  My eyes still glaze over when I have
to look at something like that.  And having, say, three convenience
functions for covering the most important applications of this general
mechanism would be only helping somewhat.

I don't want stuff like that in package code when the code is not
explicitly strictly meddling in internals of the package itself.  This
is wizard-level code which has no place whatsoever in straightforward
application coding.

The stuff for Emacs, in contrast, is a straightforward keymap menu
entry.  Yes, XEmacs programmers will all sneer at the "unclean" way
Emacs uses keymaps for toolbars, menus and whatnot.  But it keeps the
complexity manageable.  For the programmer.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: face-remapping patch

[Stephen J. Turnbull]

David Kastrup writes:

 > ;;; [Courtesy Stephen J. Turnbull, with some modifications
 > ;;;  Message-ID: <87el9fglsj.fsf@tleepslib.sk.tsukuba.ac.jp>
 > ;;;  I could not have figured this out for the world]
 > ;;; Hm, there really ought to be a way to get the spec that would be
 > ;;; instantiated in a given domain
 >   (when preview-tb-icon
 >     (let ((tb (cdadar (or (specifier-spec-list default-toolbar (current-buffer))
 > 			  (specifier-spec-list default-toolbar 'global)))))

Very likely this can be simplified to

    (when preview-tb-icon
      (let ((tb (specifier-instance default-toolbar)))

I told you how to do what you said you wanted to do.  This is
probably good enough for practical purposes, and not the kind of thing
that would frighten small children.

Re: face-remapping patch

[Richard M Stallman]

I think we should continue with local variable bindings (of various
sorts) rather than change to specifiers.  The fact that these bindings
apply to the same variables that ordinary code can look at
is a great convenience.

Re: face-remapping patch

[David Kastrup]

Richard M Stallman <rms@gnu.org> writes:

> I think we should continue with local variable bindings (of various
> sorts) rather than change to specifiers.  The fact that these bindings
> apply to the same variables that ordinary code can look at
> is a great convenience.

As I said: specifiers have the advantage of being able to
define/differentiate behavior even in the case where the locus is not
established yet.  We have a similar thing with face specifications.  We
never had this for image specifications, I think (where one can specify
in XEmacs what to do for B&W, monochrome and more complex displays).

If I get the terminology more or less right, a specifier is then
instantiated on a particular device.  That's where our localities come
into play.

I doubt that we have enough different and pressing applications for
"uninstantiated specifiers" that a generic approach is really needed.

And I don't think that conflicts in our current locality schemes will
magically disappear by switching to different data structures where we
basically tell people "sort out conflicts yourself if you understand
them".  Choice is no substitute for ease of use.

-- 
David Kastrup

Re: face-remapping patch

[Richard M Stallman]

    As I said: specifiers have the advantage of being able to
    define/differentiate behavior even in the case where the locus is not
    established yet.

In what practical case is this an advantage?

Re: face-remapping patch

[David Kastrup]

Richard M Stallman <rms@gnu.org> writes:

>     As I said: specifiers have the advantage of being able to
>     define/differentiate behavior even in the case where the locus is not
>     established yet.
>
> In what practical case is this an advantage?

Being able to define different parameters for different display types,
for example.  We have this for face definitions already.

Try
M-x customize-face RET highlight RET

then click on "State/For all kinds of displays".  You get what amounts
to a specifier of different faces that are "not yet" established.

Now for multi-tty support, on different tty's different faces will look
different.

This can be used in XEmacs for a unified approach to offering
display-dependent face, image, toolbar, menubar selections.

Which is a good idea when you are taking multi-tty support seriously
(which also might mean connecting from different computers to your local
Emacs session and having it open a terminal/frame on your external
terminal).  In practice, it is rather hard to beat application
programmers to actually provide all this versatility and work with it.

In theory, XEmacs could get along without buffer-local variables, using
specifiers instead.  In practice, enough programmers are already scared
off the areas in XEmacs which require the use of specifiers.

x-local variables seem less scary, but you can't use them (like in
customizations) before the locale x even exists.

In practice, uninstantiated specifiers might not be needed to get
manipulated outside of the customize interface in most cases.  So I
don't see we should create something like that with a life of its own.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: face-remapping patch

[Richard M Stallman]

    Being able to define different parameters for different display types,
    for example.  We have this for face definitions already.

    Try
    M-x customize-face RET highlight RET

    then click on "State/For all kinds of displays".  You get what amounts
    to a specifier of different faces that are "not yet" established.

Ok, I see what you mean.

Are there variables for which that would be a useful thing to do?
Looking at some use cases, maybe we can identify what features
would actually be useful.

    x-local variables seem less scary, but you can't use them (like in
    customizations) before the locale x even exists.

In fact, we do have such a feature for buffer-local variable bindings:
major modes, and their mode hooks.

Is there another kind of use case where we would like to be able
to specify some kind of binding values for some kind of context
in advance?

Re: face-remapping patch

[Richard M Stallman]

Your local variable feature is quite clean.

    (local-variable-p VARIABLE &optional LOCUS)

    Non-nil if VARIABLE has a local binding in locus LOCUS (see make-variable).
    BUFFER defaults to the current buffer.

If LOCUS is nil, this should mean "Is any kind of local binding now in
effect?"

Re: face-remapping patch

[David Kastrup]

Richard M Stallman <rms@gnu.org> writes:

> Your local variable feature is quite clean.

If features could be implemented just by writing DOC strings, we'd
likely have a lot of clean features.

The documentation might be clean, but that does not necessarily make the
implementation nice.

>     (local-variable-p VARIABLE &optional LOCUS)
>
>     Non-nil if VARIABLE has a local binding in locus LOCUS (see make-variable).
>     BUFFER defaults to the current buffer.
>
> If LOCUS is nil, this should mean "Is any kind of local binding now in
> effect?"

Well, that would be backwards incompatible: I actually stole half of the
descriptions from existing functions and DOC strings.

I was just describing an interface that I would be able to understand.
That does not mean that it corresponds with a simple or elegant or
efficient or flexible implementation.

If a variable has an associated vector of locuses, the elements of which
would be something like

[last-checked-selected-window
 last-checked-current-buffer
 last-checked-selected-frame
 last-checked-selected-terminal]

Where "nil" means "no local value of that kind for this variable", then
when accessing a variable, one would run through that array, and would
for the first non-nil entry not corresponding to the current locus
update the current value from some hash structure indexed via
(locus . symbol).

A list instead of a vector would be conceivable, too, (and would make
the default of variables without any local binding faster) but one would
have to think about how to order and prioritize it properly, then.

-- 
David Kastrup

Specifiers (was: face-remapping patch)

[Stefan Monnier]

>>> Miles, could you clarify why you are proposing a remapping list rather than
>>> "buffer-local faces"?
>> 
>> I think adding "buffer-local faces" directly is no better.  The good
>> thing about Miles's patch is that it leverages buffer-local variables to
>> modify faces buffer-locally.  As a result, his patch is pretty small.
>> 
>> To improve on this, I think we'd have to move to something more like
>> XEmacs's specifiers.  But I haven't even seen any proposal for such
>> a thing yet.

> Proposal: allow a third argument for make-local-variable.

For me, any such proposal which uses variables is simply a non-starter.

To undestand why, consider the following, both from the point of view of
the implementation (where variable access needs to be fast) and of the
specification of meaningful semantics:

- what should (setq var val) do?
  currently is magically uses either the global or the current-buffer
  locus depending on whether the variable is already buffer-local.
  but if it's buffer-local and frame-local, which setting should
  be changed?

- how does it interact with make-variable-buffer-local?
  I.e. if a variable is just window-local and you do (setq var val), is
  it going to be made buffer-local?

- what about `let'?  This one is really fun!
  what does
     (let ((var val)) .. (make-local-variable 'var locus) ..)
  do?  how 'bout
    (make-local-variable 'var foo) .. (let ((var val)) .. (setq var foo)?
  or
    (make-local-variable 'var foo)..(let ((var val))..(select-other-foo..)var)?


BTW, for what it's worth, I have played a tiny bit with something
similar to what you propose (except that it is separate from variables,
as you may have guessed ;-).  See sample code below.


        Stefan


;;;; Settings

;; A "setting" is the Emacs equivalent of an XEmacs "specifier".

(defvar settings--table (make-hash-table :weakness 'key))

(defun settings--put (setting value context &optional overwrite)
  (assert (not (hash-table-p value)))
  (let ((table settings--table)
        (selector setting))
    (while context
      (assert (hash-table-p table))
      (let ((entry (gethash selector table)))
        (if (hash-table-p entry)
            (setq table entry)
          (setq table (puthash selector (make-hash-table :weakness 'key) table))
          (if entry (puthash t entry table))))
      (setq selector (pop context)))
    (if overwrite
        (puthash selector value table)
      (let ((prev (gethash selector table)))
        (if (hash-table-p prev)
            (puthash t value prev)
          (puthash selector value table))))))

(defun settings--permute (xs &optional ys xss)
  (if (null xs)
      (cons ys xss)
    (dolist (x xs)
      (setq xss (settings--permute (remq x xs) (cons x ys) xss)))
    xss))
 
(defun settings-put (setting value context)
  (assert (and value (not (hash-table-p value))))
  (assert (listp context))
  (dolist (context (settings--permute context))
    (settings--put setting value context)))

(defun settings--context ()
  ;; The order indicates the precedence: a buffer-local settings takes
  ;; precedence over a window-local setting, ...
  (list (current-buffer)
        (selected-window)
        (selected-frame)
        (selected-terminal)
        major-mode))

(defun settings--get-closest (table)
  (let ((workqueue (list table)))
    (while workqueue
      (setq table (pop workqueue))
      (if (gethash t table)
          (throw 'closest (gethash t table))
        (maphash (lambda (k v)
                   (if (hash-table-p v)
                       ;; Duh!  O(N) insertion into the workqueue :-(
                       (setq worklist (append workqueue (list v)))
                     (throw 'closest v)))
                 table)))))

(defun settings--get (table context closest)
  (let (entry)
    (cond
     ((not (hash-table-p table)) table)
     ((not context)
      (or (gethash t table)
          (when closest
            ;; There is some setting somewhere in a subcontext: use it.
            (catch 'closest
              (settings--get-closest table)))))
     (t
      (or (and (setq entry (gethash (pop context) table))
               (settings--get entry context closest))
          (settings--get table context closest))))))

(defun settings-get (setting &optional context closest)
  (unless context (setq context (settings--context)))
  (let ((table (gethash setting settings--table)))
    (if (not (hash-table-p table))
        table
      (settings--get table context closest))))

Re: Specifiers

[David Kastrup]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>>> Miles, could you clarify why you are proposing a remapping list rather than
>>>> "buffer-local faces"?
>>> 
>>> I think adding "buffer-local faces" directly is no better.  The good
>>> thing about Miles's patch is that it leverages buffer-local variables to
>>> modify faces buffer-locally.  As a result, his patch is pretty small.
>>> 
>>> To improve on this, I think we'd have to move to something more like
>>> XEmacs's specifiers.  But I haven't even seen any proposal for such
>>> a thing yet.
>
>> Proposal: allow a third argument for make-local-variable.
>
> For me, any such proposal which uses variables is simply a
> non-starter.
>
> To undestand why, consider the following, both from the point of view
> of the implementation (where variable access needs to be fast) and of
> the specification of meaningful semantics:
>
> - what should (setq var val) do?
>   currently is magically uses either the global or the current-buffer
>   locus depending on whether the variable is already buffer-local.
>   but if it's buffer-local and frame-local, which setting should
>   be changed?

Of course the one that would get picked when reading the variable.  If
both bindings existed, the buffer-local one would be used with the
priority list I proposed.

> - how does it interact with make-variable-buffer-local?
>   I.e. if a variable is just window-local and you do (setq var val), is
>   it going to be made buffer-local?

"just window-local"?  I think that window-locality should have the
highest priority since one might want to have the same buffer in
different windows.  make-variable-buffer-local essentially registers a
vote for buffer-locality: with the priority list I had proposed, it
would not have an effect when you try setting a window-local variable,
but would create a buffer-local binding shadowing frame/terminal
locality otherwise.

Note that this is mostly theoretic: in practice, one would rarely want
to use different locus types for a single variable.

> - what about `let'?  This one is really fun!

`let' temporarily makes a locus-free binding.  The locality gets
restored at the end of let.  This is not different to the current
interaction of buffer-locality and let.

>   what does
>      (let ((var val)) .. (make-local-variable 'var locus) ..)
>   do?  how 'bout
>     (make-local-variable 'var foo) .. (let ((var val)) .. (setq var foo)?
>   or
>     (make-local-variable 'var foo)..(let ((var val))..(select-other-foo..)var)?

It gets messy.  But this messiness is nothing new:

(info "(elisp) Creating Buffer-Local.")


> BTW, for what it's worth, I have played a tiny bit with something
> similar to what you propose (except that it is separate from
> variables, as you may have guessed ;-).  See sample code below.

> ;;;; Settings
>
> ;; A "setting" is the Emacs equivalent of an XEmacs "specifier".

The problem is that we already have buffer-locality and frame-locality
and terminal-locality.  And the behavior of all those is currently
inconsistent.  The approach I proposed would make them consistent.
Adding something functionally similar to the existing localities but
incompatible is not really helping keeping things simple.  And as long
as not more than one locality was introduced, there would be no speed
impact for the list-based implementation.

It would also be a possibility to allow only a single locality type
which would render the priority question irrelevant.  However, I think
that the usage pattern "basically buffer-local, but overridable in some
window" would be common.

Having something almost, but not quite, entirely unlike XEmacs
specifiers duplicating the same functionality space of the existing
mechanisms -- no, I don't think we want that.

I'll readily admit that specifiers (we have face specs as well) offer
something more than my locality concept: namely uninstantiated
specifications.  But when you instantiate the stuff, namely turn it into
something minted for a specific display or frame, I think that just
making it an appropriately localized variable with a single value will
be likely the most straightforward thing to do.

If it could be done, I think it would encapsulate the complexity into a
small area of the total code, and most programmers would not have to
wrack their brain about it.

-- 
David Kastrup

Re: Specifiers

[Stefan Monnier]

> Note that this is mostly theoretic: in practice, one would rarely want
> to use different locus types for a single variable.

Until you get bug-reports.  The situation changes significantly if you
disallow combining different locus types: it's still not great
performancewise, but it's much more sane.

>> - what about `let'?  This one is really fun!

> `let' temporarily makes a locus-free binding.  The locality gets
> restored at the end of let.  This is not different to the current
> interaction of buffer-locality and let.

The current `let' behavior does not create temporarily
a locus-free binding.  E.g.

   (let ((default-directory "toto"))
     (with-current-buffer "newbuf" default-directory)

does not return "toto".

>> what does
>>   (let ((var val)) .. (make-local-variable 'var locus) ..)
>> do?  how 'bout
>>   (make-local-variable 'var foo) .. (let ((var val)) .. (setq var foo)?
>> or
>>   (make-local-variable 'var foo)..(let ((var val))..(select-other-foo..)var)?

> It gets messy.  But this messiness is nothing new:

There is something new about it, which is that as you add more types of
loci, it gets more and more difficult and costly to react to
locus-changes (e.g. set-buffer, select-window, select-frame, setq
major-mode, ...).

> The problem is that we already have buffer-locality and frame-locality
> and terminal-locality.

Indeed, that's a problem.  But frame-locality is on the way out.
Buffer-locality is here to stay, I think.

As for terminal-locality it is both less clean (because it's not really
developped, e.g. no equivalent to local-variable-p), and less
problematic because it cannot be combined with buffer-locality or
frame-locality and it is automatically local to all terminals rather
than only some, so it basically doesn't have global bindings, and it
only applies to predefined variables.

> And the behavior of all those is currently inconsistent.  The approach
> I proposed would make them consistent.

Show me a patch, so I can shoot holes through it.
Or better, don't bother: you'd be wasting your time.

> Adding something functionally similar to the existing localities but
> incompatible is not really helping keeping things simple.

You're looking for a simplicity akin to a 17-dimension "unifiying theory
of physics".  You may end up with a single very powerful and orthogonal
set of features, but I do not believe you'll get something simple.

> And as long as not more than one locality was introduced, there would
> be no speed impact for the list-based implementation.

I do not know what you're referring to here.


        Stefan

Re: Specifiers

[Stephen J. Turnbull]

David Kastrup writes:

 > Note that this is mostly theoretic: in practice, one would rarely want
 > to use different locus types for a single variable.

Huh?  In XEmacs it is done all the time, for the font, color, and
background properties of faces.

Re: Specifiers (was: face-remapping patch)

[Richard M Stallman]

    - what should (setq var val) do?
      currently is magically uses either the global or the current-buffer
      locus depending on whether the variable is already buffer-local.
      but if it's buffer-local and frame-local, which setting should
      be changed?

    - how does it interact with make-variable-buffer-local?
      I.e. if a variable is just window-local and you do (setq var val), is
      it going to be made buffer-local?

    - what about `let'?  This one is really fun!

All these issues arise already with the current features,
and won't get any worse with the proposed new feature.

ISTR that there is already code in specbind to try to DTRT
for some of these.

Re: Specifiers

[Stefan Monnier]

>     - what should (setq var val) do?
>       currently is magically uses either the global or the current-buffer
>       locus depending on whether the variable is already buffer-local.
>       but if it's buffer-local and frame-local, which setting should
>       be changed?

>     - how does it interact with make-variable-buffer-local?
>       I.e. if a variable is just window-local and you do (setq var val), is
>       it going to be made buffer-local?

>     - what about `let'?  This one is really fun!

> All these issues arise already with the current features,

Yes, pretty much, except we don't have to worry about combinations of
buffer-local with other forms of foo-local at the same time (well,
supposedly we have to worry about it for buffer-local + frame-local,
but we already know we don't handle that correctly).

> and won't get any worse with the proposed new feature.

Try it.

> ISTR that there is already code in specbind to try to DTRT
> for some of these.

Yes, there's already a fair bit of code trying to get things to work
right for buffer-local variables (in specbind and a bunch of other
places).  It's already pretty complex just like that.


        Stefan

Re: Specifiers

[David Kastrup]

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

> Yes, pretty much, except we don't have to worry about combinations of
> buffer-local with other forms of foo-local at the same time (well,
> supposedly we have to worry about it for buffer-local + frame-local,
> but we already know we don't handle that correctly).

That's supposed to be an advantage?

>> and won't get any worse with the proposed new feature.
>
> Try it.
>
>> ISTR that there is already code in specbind to try to DTRT
>> for some of these.
>
> Yes, there's already a fair bit of code trying to get things to work
> right for buffer-local variables (in specbind and a bunch of other
> places).  It's already pretty complex just like that.

I don't quite see what handling independent heaps of complexity for the
three kinds of locality we currently support (and window-locality is
about to come, too, even though for faces, _another_ complexity source
handled independently at the moment) is supposed to be buying us.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Specifiers

[Stefan Monnier]

>> Yes, pretty much, except we don't have to worry about combinations of
>> buffer-local with other forms of foo-local at the same time (well,
>> supposedly we have to worry about it for buffer-local + frame-local,
>> but we already know we don't handle that correctly).
> That's supposed to be an advantage?

What does "That" refer to?
Not having to worry about combinations is an advantage, yes.

> I don't quite see what handling independent heaps of complexity for the
> three kinds of locality we currently support (and window-locality is
> about to come, too, even though for faces, _another_ complexity source
> handled independently at the moment) is supposed to be buying us.

Ever heard of "divide and conquer"?
In any case, try to write the code, and you'll quickly see what it is
buying us.


        Stefan

Re: Specifiers

[David Kastrup]

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

>>> Yes, pretty much, except we don't have to worry about combinations of
>>> buffer-local with other forms of foo-local at the same time (well,
>>> supposedly we have to worry about it for buffer-local + frame-local,
>>> but we already know we don't handle that correctly).
>> That's supposed to be an advantage?
>
> What does "That" refer to?
> Not having to worry about combinations is an advantage, yes.

Knowing that we don't handle something correctly already is an advantage
over a scheme that has a chance of offering well-defined behavior?

-- 
David Kastrup

Re: Specifiers

[Stefan Monnier]

>>>> Yes, pretty much, except we don't have to worry about combinations of
>>>> buffer-local with other forms of foo-local at the same time (well,
>>>> supposedly we have to worry about it for buffer-local + frame-local,
>>>> but we already know we don't handle that correctly).
>>> That's supposed to be an advantage?
>> 
>> What does "That" refer to?
>> Not having to worry about combinations is an advantage, yes.

> Knowing that we don't handle something correctly already is an advantage
> over a scheme that has a chance of offering well-defined behavior?

I think you misunderstood.  What is happening is that you're suggesting
to make such combinations even more complex and common, while we already
aren't even able to handle them right in the limited cases that
can currently arise.


        Stefan

Re: Specifiers

[Richard M Stallman]

    I think you misunderstood.  What is happening is that you're suggesting
    to make such combinations even more complex and common, while we already
    aren't even able to handle them right in the limited cases that
    can currently arise.

Is there a real practical problem with the cases that we
"don't handle right"?

By what standard is the current handling "not right"?

Re: face-remapping patch

[Stefan Monnier]

>> To improve on this, I think we'd have to move to something more like
>> XEmacs's specifiers.  But I haven't even seen any proposal for such
>> a thing yet.

> Proposal: allow a third argument for make-local-variable.

That's for variables, not faces.
But, yes, maybe we should rethink face-settings along similar lines:

  (set-face-attribute FACE LOCUS &rest ARGS)

The first difficulty is to figure out how to integrate such a thing with
the settings coming from X resources, from defface specs, from frame
parameters (font & back/foreground colors, IIRC), ...

Note that it would be a good opportunity to try and make this whole code
more efficient as well, so that all face settings don't get redundantly
recomputed everytime we create a new frame.

I guess the defface spec and the X resources could be turned into
terminal-local settings computed everytime a new terminal is created
(and everytime a new face is created, of course).


        Stefan

Re: face-remapping patch

[David Kastrup]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> To improve on this, I think we'd have to move to something more like
>>> XEmacs's specifiers.  But I haven't even seen any proposal for such
>>> a thing yet.
>
>> Proposal: allow a third argument for make-local-variable.
>
> That's for variables, not faces.

Yes, quite so.  I was trying to brainstorm about one mechanism at a
time.  I am not convinced that it is a good idea to treat faces
completely differently.

> But, yes, maybe we should rethink face-settings along similar lines:
>
>   (set-face-attribute FACE LOCUS &rest ARGS)
>
> The first difficulty is to figure out how to integrate such a thing
> with the settings coming from X resources, from defface specs, from
> frame parameters (font & back/foreground colors, IIRC), ...
>
> Note that it would be a good opportunity to try and make this whole
> code more efficient as well, so that all face settings don't get
> redundantly recomputed everytime we create a new frame.
>
> I guess the defface spec and the X resources could be turned into
> terminal-local settings computed everytime a new terminal is created
> (and everytime a new face is created, of course).

Something like that.  We already have too many different mechanisms at
work for similar things.  Unifying the approaches (even if we find out
that unifying the underlying internals is not feasible) would, I think,
in general be helpful for application programmers.

-- 
David Kastrup

Re: face-remapping patch

[Miles Bader]

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

Here's a new patch addressing your comments.

-Miles

-- 
Custard, n. A vile concoction produced by a malevolent conspiracy of the hen,
the cow, and the cook.

[-- Attachment #2: face-remapping-20080529-0.patch --]
[-- Type: text/plain, Size: 22209 bytes --]

orig = emacs@sv.gnu.org/emacs--devo--0--patch-1183
mod = emacs@sv.gnu.org/emacs--devo--0--patch-1183

A  src/ChangeLog.face-remap
M  src/xfaces.c
M  src/dispextern.h
M  src/fontset.c
M  src/xdisp.c
M  doc/lispref/display.texi

--- orig/doc/lispref/display.texi
+++ mod/doc/lispref/display.texi
@@ -2365,6 +2365,58 @@
   When multiple overlays cover one character, an overlay with higher
 priority overrides those with lower priority.  @xref{Overlays}.
 
+@defvar face-remapping-alist
+@tindex face-remapping-alist
+  This variable is used for buffer-local changes in the appearance of
+a face, for instance making the @code{default} face a variable-pitch
+face in a particular buffer.
+
+  Its value should be an alist, whose elements have the form
+@code{(@var{face} @var{remapping}@dots{})}; when the text specifies
+face @var{face}, Emacs redisplay uses @var{remapping}@dots{} instead.
+@var{remapping}@dots{} may be any face specification suitable for a
+@code{face} text property, usually a face name, but also perhaps a
+property list of face attribute/value pairs; @xref{Special
+Properties}.
+
+Two points bear emphasizing:
+
+@enumerate
+@item
+The new definition @var{remapping}@dots{} is the complete
+specification of how to display @var{face}---it entirely replaces,
+rather than augmenting or modifying, the normal definition of that
+face.
+
+@item
+If @var{remapping}@dots{} recursively references the same face name
+@var{face}, either directly remapping entry, or via the
+@code{:inherit} attribute of some other face in
+@var{remapping}@dots{}, then that reference uses normal frame-wide
+definition of @var{face} instead of the `remapped' definition.
+
+For instance, if the @code{mode-line} face is remapped using this
+entry in @code{face-remapping-alist}:
+@example
+(mode-line italic mode-line)
+@end example
+Then the new definition of the @code{mode-line} face inherits from the
+@code{italic} face, and the @emph{normal} (non-remapped) definition of
+@code{mode-line} face.
+@end enumerate
+
+  A typical use of the @code{face-remapping-alist} is to change a
+buffer's @code{default} face; for example, the following changes a
+buffer's @code{default} face to use the @code{variable-pitch} face,
+with the height doubled:
+
+@example
+(set (make-local-variable 'face-remapping-alist)
+     '((default variable-pitch :height 2.0)))
+@end example
+
+@end defvar
+
 @node Font Selection
 @subsection Font Selection
 


--- orig/src/dispextern.h
+++ mod/src/dispextern.h
@@ -2852,6 +2852,7 @@
 int lookup_face P_ ((struct frame *, Lisp_Object *));
 int lookup_non_ascii_face P_ ((struct frame *, 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));
@@ -2880,6 +2881,8 @@
 extern Lisp_Object split_font_name_into_vector P_ ((Lisp_Object));
 extern Lisp_Object build_font_name_from_vector P_ ((Lisp_Object));
 
+extern Lisp_Object Vface_remapping_alist;
+
 /* Defined in xfns.c  */
 
 #ifdef HAVE_X_WINDOWS


--- orig/src/fontset.c
+++ mod/src/fontset.c
@@ -1727,7 +1727,7 @@
       CHECK_CHARACTER (ch);
       c = XINT (ch);
       f = XFRAME (selected_frame);
-      face_id = DEFAULT_FACE_ID;
+      face_id = lookup_basic_face (f, DEFAULT_FACE_ID);
       pos = -1;
       cs_id = -1;
     }


--- orig/src/xdisp.c
+++ mod/src/xdisp.c
@@ -2493,6 +2493,7 @@
      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);
@@ -2509,6 +2510,10 @@
       free_all_realized_faces (Qnil);
     }
 
+  /* Perhaps remap BASE_FACE_ID to a user-specified alternative.  */
+  if (! NILP (Vface_remapping_alist))
+    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)
@@ -2524,7 +2529,7 @@
   bzero (it, sizeof *it);
   it->current.overlay_string_index = -1;
   it->current.dpvec_index = -1;
-  it->base_face_id = base_face_id;
+  it->base_face_id = remapped_base_face_id;
   it->string = Qnil;
   IT_STRING_CHARPOS (*it) = IT_STRING_BYTEPOS (*it) = -1;
 
@@ -2709,11 +2714,11 @@
     {
       struct face *face;
 
-      it->face_id = base_face_id;
+      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, base_face_id);
+      face = FACE_FROM_ID (it->f, remapped_base_face_id);
       if (face->box != FACE_NO_BOX)
 	it->start_of_box_run_p = 1;
     }
@@ -4079,7 +4084,8 @@
 	      /* Value is a multiple of the canonical char height.  */
 	      struct face *face;
 
-	      face = FACE_FROM_ID (it->f, DEFAULT_FACE_ID);
+	      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]));
 	    }
@@ -4189,7 +4195,7 @@
 	  || EQ (XCAR (spec), Qright_fringe))
       && CONSP (XCDR (spec)))
     {
-      int face_id = DEFAULT_FACE_ID;
+      int face_id = lookup_basic_face (it->f, DEFAULT_FACE_ID);
       int fringe_bitmap;
 
       if (!FRAME_WINDOW_P (it->f))


--- orig/src/xfaces.c
+++ mod/src/xfaces.c
@@ -422,6 +422,23 @@
 
 Lisp_Object Vface_new_frame_defaults;
 
+/* Alist of face remappings.  Each element is of the form:
+   (FACE REPLACEMENT...) which causes display of the face FACE to use
+   REPLACEMENT... instead.  REPLACEMENT... is interpreted the same way
+   the value of a `face' text property is: it may be (1) A face name,
+   (2) A list of face names, (3) A property-list of face attribute/value
+   pairs, or (4) A list of face names intermixed with lists containing
+   face attribute/value pairs.
+
+   Multiple entries in REPLACEMENT... are merged together to form the final
+   result, with faces or attributes earlier in the list taking precedence
+   over those that are later.
+
+   Face-name remapping cycles are suppressed; recursive references use
+   the underlying face instead of the remapped face.  */
+
+Lisp_Object Vface_remapping_alist;
+
 /* The next ID to assign to Lisp faces.  */
 
 static int next_lface_id;
@@ -493,7 +510,8 @@
 static Lisp_Object resolve_face_name P_ ((Lisp_Object, int));
 static int may_use_scalable_font_p P_ ((const char *));
 static void set_font_frame_param P_ ((Lisp_Object, Lisp_Object));
-static int get_lface_attributes P_ ((struct frame *, Lisp_Object, Lisp_Object *, int));
+static int get_lface_attributes P_ ((struct frame *, Lisp_Object, Lisp_Object *,
+				     int, struct named_merge_point *));
 static int load_pixmap P_ ((struct frame *, Lisp_Object, unsigned *, unsigned *));
 static unsigned char *xstrlwr P_ ((unsigned char *));
 static struct frame *frame_or_selected_frame P_ ((Lisp_Object, int));
@@ -2058,6 +2076,12 @@
 \f
 /* Face-merge cycle checking.  */
 
+enum named_merge_point_kind
+{
+  NAMED_MERGE_POINT_NORMAL,
+  NAMED_MERGE_POINT_REMAP
+};
+
 /* A `named merge point' is simply a point during face-merging where we
    look up a face by name.  We keep a stack of which named lookups we're
    currently processing so that we can easily detect cycles, using a
@@ -2067,27 +2091,40 @@
 struct named_merge_point
 {
   Lisp_Object face_name;
+  enum named_merge_point_kind named_merge_point_kind;
   struct named_merge_point *prev;
 };
 
 
 /* If a face merging cycle is detected for FACE_NAME, return 0,
    otherwise add NEW_NAMED_MERGE_POINT, which is initialized using
-   FACE_NAME, as the head of the linked list pointed to by
-   NAMED_MERGE_POINTS, and return 1.  */
+   FACE_NAME and NAMED_MERGE_POINT_KIND, as the head of the linked list
+   pointed to by NAMED_MERGE_POINTS, and return 1.  */
 
 static INLINE int
 push_named_merge_point (struct named_merge_point *new_named_merge_point,
 			Lisp_Object face_name,
+			enum named_merge_point_kind named_merge_point_kind,
 			struct named_merge_point **named_merge_points)
 {
   struct named_merge_point *prev;
 
   for (prev = *named_merge_points; prev; prev = prev->prev)
     if (EQ (face_name, prev->face_name))
-      return 0;
+      {
+	if (prev->named_merge_point_kind == named_merge_point_kind)
+	  /* A cycle, so fail.  */
+	  return 0;
+	else if (prev->named_merge_point_kind == NAMED_MERGE_POINT_REMAP)
+	  /* A remap `hides ' any previous normal merge points
+	     (because the remap means that it's actually different face),
+	     so as we know the current merge point must be normal, we
+	     can just assume it's OK.  */
+	  break;
+      }
 
   new_named_merge_point->face_name = face_name;
+  new_named_merge_point->named_merge_point_kind = named_merge_point_kind;
   new_named_merge_point->prev = *named_merge_points;
 
   *named_merge_points = new_named_merge_point;
@@ -2165,22 +2202,17 @@
 /* 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.  */
-
+   face text properties; Ediff uses that).  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)
+lface_from_face_name_no_resolve (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, signal_p);
-
   if (f)
     lface = assq_no_quit (face_name, f->face_alist);
   else
@@ -2192,9 +2224,28 @@
     signal_error ("Invalid face", face_name);
 
   check_lface (lface);
+
   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;
+{
+  face_name = resolve_face_name (face_name, signal_p);
+  return lface_from_face_name_no_resolve (f, face_name, signal_p);
+}
+
 
 /* Get face attributes of face FACE_NAME from frame-local faces on
    frame F.  Store the resulting attributes in ATTRS which must point
@@ -2203,26 +2254,65 @@
    Otherwise, value is zero if FACE_NAME is not a face.  */
 
 static INLINE int
-get_lface_attributes (f, face_name, attrs, signal_p)
+get_lface_attributes_no_remap (f, face_name, attrs, signal_p)
      struct frame *f;
      Lisp_Object face_name;
      Lisp_Object *attrs;
      int signal_p;
 {
   Lisp_Object lface;
-  int success_p;
 
-  lface = lface_from_face_name (f, face_name, signal_p);
-  if (!NILP (lface))
+  lface = lface_from_face_name_no_resolve (f, face_name, signal_p);
+
+  if (! NILP (lface))
+    bcopy (XVECTOR (lface)->contents, attrs,
+	   LFACE_VECTOR_SIZE * sizeof *attrs);
+
+  return !NILP (lface);
+}
+
+/* Get face attributes of face FACE_NAME from frame-local faces on frame
+   F.  Store the resulting attributes in ATTRS which must point to a
+   vector of Lisp_Objects of size LFACE_VECTOR_SIZE.  If FACE_NAME is an
+   alias for another face, use that face's definition.  If SIGNAL_P is
+   non-zero, signal an error if FACE_NAME does not name a face.
+   Otherwise, value is zero if FACE_NAME is not a face.  */
+
+static INLINE int
+get_lface_attributes (f, face_name, attrs, signal_p, named_merge_points)
+     struct frame *f;
+     Lisp_Object face_name;
+     Lisp_Object *attrs;
+     int signal_p;
+     struct named_merge_point *named_merge_points;
+{
+  Lisp_Object face_remapping;
+
+  face_name = resolve_face_name (face_name, signal_p);
+
+  /* See if SYMBOL has been remapped to some other face (usually this
+     is done buffer-locally).  */
+  face_remapping = assq_no_quit (face_name, Vface_remapping_alist);
+  if (CONSP (face_remapping))
     {
-      bcopy (XVECTOR (lface)->contents, attrs,
-	     LFACE_VECTOR_SIZE * sizeof *attrs);
-      success_p = 1;
+      struct named_merge_point named_merge_point;
+
+      if (push_named_merge_point (&named_merge_point,
+				  face_name, NAMED_MERGE_POINT_REMAP,
+				  &named_merge_points))
+	{
+	  int i;
+
+	  for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
+	    attrs[i] = Qunspecified;
+
+	  return merge_face_ref (f, XCDR (face_remapping), attrs,
+				 signal_p, named_merge_points);
+	}
     }
-  else
-    success_p = 0;
 
-  return success_p;
+  /* Default case, no remapping.  */
+  return get_lface_attributes_no_remap (f, face_name, attrs, signal_p);
 }
 
 
@@ -2378,8 +2468,8 @@
    specified attribute of FROM overrides the corresponding attribute of
    TO; relative attributes in FROM are merged with the absolute value in
    TO and replace it.  NAMED_MERGE_POINTS is used internally to detect
-   loops in face inheritance; it should be 0 when called from other
-   places.  */
+   loops in face inheritance/remapping; it should be 0 when called from
+   other places.  */
 
 static INLINE void
 merge_face_vectors (f, from, to, named_merge_points)
@@ -2454,11 +2544,12 @@
   struct named_merge_point named_merge_point;
 
   if (push_named_merge_point (&named_merge_point,
-			      face_name, &named_merge_points))
+			      face_name, NAMED_MERGE_POINT_NORMAL,
+			      &named_merge_points))
     {
       struct gcpro gcpro1;
       Lisp_Object from[LFACE_VECTOR_SIZE];
-      int ok = get_lface_attributes (f, face_name, from, 0);
+      int ok = get_lface_attributes (f, face_name, from, 0, named_merge_points);
 
       if (ok)
 	{
@@ -3436,7 +3527,7 @@
 
       /* Changing the background color might change the background
 	 mode, so that we have to load new defface specs.
-	 Call frame-set-background-mode to do that.  */
+	 Call frame-update-face-colors to do that.  */
       XSETFRAME (frame, f);
       call1 (Qframe_set_background_mode, frame);
 
@@ -4642,7 +4733,7 @@
 	abort ();  /* realize_basic_faces must have set it up  */
     }
 
-  if (!get_lface_attributes (f, symbol, symbol_attrs, signal_p))
+  if (! get_lface_attributes (f, symbol, symbol_attrs, signal_p, 0))
     return -1;
 
   bcopy (default_face->lface, attrs, sizeof attrs);
@@ -4652,6 +4743,58 @@
 }
 
 
+/* 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_remapping_alist.  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_remapping_alist))
+    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;
+    case TOOL_BAR_FACE_ID:		name = Qtool_bar;		break;
+    case FRINGE_FACE_ID:		name = Qfringe;			break;
+    case SCROLL_BAR_FACE_ID:		name = Qscroll_bar;		break;
+    case BORDER_FACE_ID:		name = Qborder;			break;
+    case CURSOR_FACE_ID:		name = Qcursor;			break;
+    case MOUSE_FACE_ID:			name = Qmouse;			break;
+    case MENU_FACE_ID:			name = Qmenu;			break;
+
+    default:
+      abort ();	    /* the caller is supposed to pass us a basic face id */
+    }
+
+  /* Do a quick scan through Vface_remapping_alist, and return immediately
+     if there is no remapping for face NAME.  This is just an optimization
+     for the very common no-remapping case.  */
+  mapping = assq_no_quit (name, Vface_remapping_alist);
+  if (NILP (mapping))
+    return face_id;		/* Give up.  */
+
+  /* If there is a remapping entry, lookup the face using NAME, which will
+     handle the remapping too.  */
+  remapped_face_id = lookup_named_face (f, name, 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.  */
 
@@ -4784,7 +4927,7 @@
   if (!default_face)
     abort ();
 
-  get_lface_attributes (f, symbol, symbol_attrs, signal_p);
+  get_lface_attributes (f, symbol, symbol_attrs, signal_p, 0);
   bcopy (default_face->lface, attrs, sizeof attrs);
   merge_face_vectors (f, symbol_attrs, attrs, 0);
   return lookup_face (f, attrs);
@@ -5493,7 +5636,7 @@
   struct face *new_face;
 
   /* The default face must exist and be fully specified.  */
-  get_lface_attributes (f, Qdefault, attrs, 1);
+  get_lface_attributes_no_remap (f, Qdefault, attrs, 1);
   check_lface_attrs (attrs);
   xassert (lface_fully_specified_p (attrs));
 
@@ -5506,7 +5649,7 @@
     }
 
   /* Merge SYMBOL's face with the default face.  */
-  get_lface_attributes (f, symbol, symbol_attrs, 1);
+  get_lface_attributes_no_remap (f, symbol, symbol_attrs, 1);
   merge_face_vectors (f, symbol_attrs, attrs, 0);
 
   /* Realize the face.  */
@@ -6063,13 +6206,18 @@
 
   *endptr = endpos;
 
-  default_face = FACE_FROM_ID (f, DEFAULT_FACE_ID);
+
+  /* Perhaps remap BASE_FACE_ID to a user-specified alternative.  */
+  if (NILP (Vface_remapping_alist))
+    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;
+    return default_face->id;
 
   /* Begin with attributes from the default face.  */
   bcopy (default_face->lface, attrs, sizeof attrs);
@@ -6668,6 +6816,43 @@
 ignore.  */);
   Vface_ignored_fonts = Qnil;
 
+  DEFVAR_LISP ("face-remapping-alist", &Vface_remapping_alist,
+	       doc: /* Alist of face remappings.
+Each element is of the form:
+
+   (FACE REPLACEMENT...),
+
+which causes display of the face FACE to use REPLACEMENT... instead.
+REPLACEMENT... is interpreted the same way the value of a `face' text
+property is: it may be (1) A face name, (2) A list of face names, (3) A
+property-list of face attribute/value pairs, or (4) A list of face names
+intermixed with lists containing face attribute/value pairs.
+
+Multiple entries in REPLACEMENT... are merged together to form the final
+result, with faces or attributes earlier in the list taking precedence
+over those that are later.
+
+Face-name remapping cycles are suppressed; recursive references use the
+underlying face instead of the remapped face.  So a remapping of the form:
+
+   (FACE EXTRA-FACE... FACE)
+
+or:
+
+   (FACE (FACE-ATTR VAL ...) FACE)
+
+will cause EXTRA-FACE... or (FACE-ATTR VAL ...) to be _merged_ with the
+existing definition of FACE.  Note that for the default face, this isn't
+necessary, as every face inherits from the default face.
+
+Making this variable buffer-local is a good way to allow buffer-specific
+face definitions.  For instance, the mode my-mode could define a face
+`my-mode-default', and then in the mode setup function, do:
+
+   (set (make-local-variable 'face-remapping-alist)
+        '((default my-mode-default)))).  */);
+  Vface_remapping_alist = 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



* added files

--- /dev/null
+++ mod/src/ChangeLog.face-remap
@@ -0,0 +1,35 @@
+2008-05-29  Miles Bader  <miles@gnu.org>
+
+	* xfaces.c (Vface_remapping_alist): New variable.
+	(syms_of_xfaces): Initialize it.
+	(enum named_merge_point_kind): New type.
+	(struct named_merge_point): Add `named_merge_point_kind' field.
+	(push_named_merge_point): Make cycle detection respect different
+	named-merge-point kinds.
+	(lface_from_face_name_no_resolve): Renamed from `lface_from_face_name'.
+	Remove face-name alias resolution.
+	(lface_from_face_name): New definition using
+	`lface_from_face_name_no_resolve'.
+	(get_lface_attributes_no_remap): Renamed from `get_lface_attributes'.
+	Call lface_from_face_name_no_resolve instead of lface_from_face_name.
+	(get_lface_attributes): New definition that layers face-remapping on
+	top of get_lface_attributes_no_remap.  New arg `named_merge_points'.
+	(lookup_basic_face): New function.
+	(lookup_derived_face): Pass new last arg to `get_lface_attributes'.
+	(realize_named_face): Call `get_lface_attributes_no_remap' instead of
+	`get_lface_attributes'.
+	(face_at_buffer_position): Use `lookup_basic_face' to lookup
+	DEFAULT_FACE_ID if necessary.  When optimizing the default-face case,
+	return default_face's face-id instead of the constant DEFAULT_FACE_ID.
+
+	* xdisp.c (init_iterator): Pass base_face_id through
+	`lookup_basic_face' when we actually use it as a face-id.
+	(handle_single_display_prop): Use `lookup_basic_face' to lookup
+	DEFAULT_FACE_ID.
+
+	* fontset.c (Finternal_char_font): Use `lookup_basic_face' to
+	lookup the initial face-id.
+
+	* dispextern.h (lookup_basic_face, Vface_remapping_alist): New decls.
+
+;; arch-tag: aded4c61-a649-4b57-9c93-5408933a7b40

Re: face-remapping patch

[Stefan Monnier]

> Here's a new patch addressing your comments.

Looks OK to me.  Any objection?


        Stefan

Re: face-remapping patch

[Eli Zaretskii]

> Date: Thu, 29 May 2008 04:47:30 -0400
> From: Miles Bader <miles@gnu.org>
> Cc: Emacs-Devel <emacs-devel@gnu.org>
> 
> Here's a new patch addressing your comments.

Not my comments, but allow me a few remarks on the manual bits.

> +@tindex face-remapping-alist

Are we still using @tindex?  I thought it was not needed anymore a
long time ago, and should not be used.  I don't see it anywhere in the
ELisp manual in CVS trunk.

> +  This variable is used for buffer-local changes in the appearance of
> +a face, for instance making the @code{default} face a variable-pitch
> +face in a particular buffer.
> +
> +  Its value should be an alist, whose elements have the form
> +@code{(@var{face} @var{remapping}@dots{})}; when the text specifies
> +face @var{face}, Emacs redisplay uses @var{remapping}@dots{} instead.

It took me a few moments to understand what you meant by "when text
specifies face".  I finally realized that you are talking about text
properties, so please add the word "property" here, because in Emacs,
text does not specify anything except itself.

> +@var{remapping}@dots{} may be any face specification suitable for a

Using @dots{} in the middle of text will cause TeX render it as an end
of a sentence, I think.  At least put a @: after it, if you must use
it (I'd recommend against that).

> +@code{face} text property, usually a face name, but also perhaps a
> +property list of face attribute/value pairs; @xref{Special
> +Properties}.

It's not a good idea to use @xref in the middle of a sentence: it
produces capitalized "See" which is incorrect English.  "see @ref" is
a better alternative; or just start a new sentence.

> +The new definition @var{remapping}@dots{} is the complete
> +specification of how to display @var{face}---it entirely replaces,
> +rather than augmenting or modifying, the normal definition of that
> +face.

Isn't "augments or modifies" more correct, English-wise (because it
matches "replaces" better)?

> +If @var{remapping}@dots{} recursively references the same face name
> +@var{face}, either directly remapping entry, or via the
> +@code{:inherit} attribute of some other face in
> +@var{remapping}@dots{}, then that reference uses normal frame-wide
> +definition of @var{face} instead of the `remapped' definition.
                                            ^^^^^^^^
There should never be a need to use `...' in Texinfo.  Either you want
``...'', for quoting, or you want @samp.  I think you want the former
here.

> +For instance, if the @code{mode-line} face is remapped using this
> +entry in @code{face-remapping-alist}:
> +@example
> +(mode-line italic mode-line)
> +@end example
> +Then the new definition of the @code{mode-line} face inherits from the
   ^^^^
This "Then" should not begin with a capital letter, since it continues
the sentence before the @example, and you want an @outdent before it,
to prevent automatic indentation of each new paragraph.

Re: face-remapping patch

[Miles Bader]

Eli Zaretskii <eliz@gnu.org> writes:
> Not my comments, but allow me a few remarks on the manual bits.

Thanks Eli, I'll fix up the text accordingly.

-Miles

-- 
Patience, n. A minor form of despair, disguised as a virtue.

Re: face-remapping patch

[Chong Yidong]

"Miles Bader" <miles@gnu.org> writes:

> I've attached my face-remapping patch to this message (I needed to
> untangle it from some other changes).
>
> I've used regularly in my personal emacs for many years, and in light
> testing the patch seems to work fine against the current trunk.

Looks reasonable, but could you explain what problem this solves?  Under
what circumstances would a user or an Elisp library prefer to use
face-remapping instead of redefining faces directly?

Re: face-remapping patch

[David Reitter]

On 28 May 2008, at 15:46, Chong Yidong wrote:
>
> Looks reasonable, but could you explain what problem this solves?   
> Under
> what circumstances would a user or an Elisp library prefer to use
> face-remapping instead of redefining faces directly?

I like to edit LaTeX code and other texts with a different (variable- 
width) font, but I'll stick to a monospaced one for code.
Also, I like to change the background color (and a bunch of further  
faces) so I can more easily see what buffer I'm in.

These things are obviously buffer-specific.

I've been using code that changes faces and frame parameters from  
`change-major-mode-hook', but having buffer-specific faces implemented  
at C level is obviously a much cleaner solution.

Re: face-remapping patch

[Miles Bader]

Chong Yidong <cyd@stupidchicken.com> writes:
>> I've attached my face-remapping patch to this message (I needed to
>> untangle it from some other changes).
>>
>> I've used regularly in my personal emacs for many years, and in light
>> testing the patch seems to work fine against the current trunk.
>
> Looks reasonable, but could you explain what problem this solves?  Under
> what circumstances would a user or an Elisp library prefer to use
> face-remapping instead of redefining faces directly?

What I personally use it for most of the time is a little
`variable-pitch-mode' minor mode, which makes the text in the current
buffer display text using the variable-pitch face.

It's sort of a perennial feature request too ... "Can I somehow
re-define face XXX in only a single buffer?"

-Miles

-- 
Bride, n. A woman with a fine prospect of happiness behind her.

Re: face-remapping patch

[Chong Yidong]

Miles Bader <miles@gnu.org> writes:

> Chong Yidong <cyd@stupidchicken.com> writes:
>>> I've attached my face-remapping patch to this message (I needed to
>>> untangle it from some other changes).
>>>
>>> I've used regularly in my personal emacs for many years, and in light
>>> testing the patch seems to work fine against the current trunk.
>>
>> Looks reasonable, but could you explain what problem this solves?  Under
>> what circumstances would a user or an Elisp library prefer to use
>> face-remapping instead of redefining faces directly?
>
> What I personally use it for most of the time is a little
> `variable-pitch-mode' minor mode, which makes the text in the current
> buffer display text using the variable-pitch face.
>
> It's sort of a perennial feature request too ... "Can I somehow
> re-define face XXX in only a single buffer?"

I see.  I think the patch makes good sense.

BTW, off the top of your head, is any existing code in Emacs that could
benefit from using face remapping?  If so, this would be a good time to
change that too.

Re: face-remapping patch

[Miles Bader]

Chong Yidong <cyd@stupidchicken.com> writes:
> I see.  I think the patch makes good sense.

Thanks  :-)

Since both maintainers seem to approve, and there have been no
substantive objections, I'll commit the patch.

-Miles

-- 
Arrest, v. Formally to detain one accused of unusualness.

Re: face-remapping patch

[Dan Nicolaescu]

"Miles Bader" <miles@gnu.org> writes:

  > I've attached my face-remapping patch to this message (I needed to
  > untangle it from some other changes).
  > 
  > I've used regularly in my personal emacs for many years, and in light
  > testing the patch seems to work fine against the current trunk.

Does this work if the buffer is displayed in 2 frames, let's say a tty
frame and an X11 frame?

Re: face-remapping patch

[Miles Bader]

Dan Nicolaescu <dann@ics.uci.edu> writes:
>   > I've used regularly in my personal emacs for many years, and in light
>   > testing the patch seems to work fine against the current trunk.
>
> Does this work if the buffer is displayed in 2 frames, let's say a tty
> frame and an X11 frame?

Sure.

-Miles

-- 
x
y
Z!