Cleaning up `unspecified' for new frames

[Chong Yidong]

Regarding the FOR-RELEASE item:

  ** Clean up the confusion about what `unspecified' means in the face
  defaults for new frames.

  Emacs actually has two places to get the face specifications for new
  frames.  There is face-new-frame-defaults, and there is whatever the
  defface specified.

  The face-new-frame-defaults are merged in by
  Finternal_merge_in_global_face, and normally override what the
  defface specified.  But this happens only when the value in
  face-new-frame-defaults is not `unspecified'.

  In other words, setting an attribute to `unspecified' has an
  inconsistent meaning.  For an existing frame, it overrides both the
  face-new-frame-defaults and the defface.  But when applied to the
  "default for new frames", it really means that
  face-new-frame-defaults should no longer override the defface.

Here is one idea for solving it: define a new symbol, `inherit', that
can be passed to set-face-attributes -- so it will only show up in
face-new-frame-defaults, but never the defface specs.  In
merge_face_vectors, the code writes `unspecified' into the face
attribute whenever it sees `inherit'.  (It doesn't write anything when
it sees `unspecified', because that's the normal meaning of
`unspecified'.)

Any thoughts?

*** emacs/src/xfaces.c.~1.338.~ 2005-11-02 22:17:15.000000000 -0500
--- emacs/src/xfaces.c  2005-11-10 22:09:13.000000000 -0500
***************
*** 274,279 ****
--- 274,283 ----
  
  #define UNSPECIFIEDP(ATTR) EQ ((ATTR), Qunspecified)
  
+ /* Non-zero if face attribute ATTR is `inherit'.  */
+ 
+ #define INHERITP(ATTR) EQ ((ATTR), Qinherit)
+ 
  /* Value is the number of elements of VECTOR.  */
  
  #define DIM(VECTOR) (sizeof (VECTOR) / sizeof *(VECTOR))
***************
*** 312,317 ****
--- 316,322 ----
  Lisp_Object Qreleased_button, Qpressed_button;
  Lisp_Object QCstyle, QCcolor, QCline_width;
  Lisp_Object Qunspecified;
+ Lisp_Object Qinherit;
  
  char unspecified_fg[] = "unspecified-fg", unspecified_bg[] = "unspecified-bg";
  
***************
*** 3104,3151 ****
--- 3109,3172 ----
       Lisp_Object *attrs;
  {
    xassert (UNSPECIFIEDP (attrs[LFACE_FAMILY_INDEX])
+          || INHERITP (attrs[LFACE_FAMILY_INDEX])
           || STRINGP (attrs[LFACE_FAMILY_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_SWIDTH_INDEX])
+          || INHERITP (attrs[LFACE_SWIDTH_INDEX])
           || SYMBOLP (attrs[LFACE_SWIDTH_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_AVGWIDTH_INDEX])
+          || INHERITP (attrs[LFACE_AVGWIDTH_INDEX])
           || INTEGERP (attrs[LFACE_AVGWIDTH_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_HEIGHT_INDEX])
+          || INHERITP (attrs[LFACE_HEIGHT_INDEX])
           || INTEGERP (attrs[LFACE_HEIGHT_INDEX])
           || FLOATP (attrs[LFACE_HEIGHT_INDEX])
           || FUNCTIONP (attrs[LFACE_HEIGHT_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_WEIGHT_INDEX])
+          || INHERITP (attrs[LFACE_WEIGHT_INDEX])
           || SYMBOLP (attrs[LFACE_WEIGHT_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_SLANT_INDEX])
+          || INHERITP (attrs[LFACE_SLANT_INDEX])
           || SYMBOLP (attrs[LFACE_SLANT_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_UNDERLINE_INDEX])
+          || INHERITP (attrs[LFACE_UNDERLINE_INDEX])
           || SYMBOLP (attrs[LFACE_UNDERLINE_INDEX])
           || STRINGP (attrs[LFACE_UNDERLINE_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
+          || INHERITP (attrs[LFACE_OVERLINE_INDEX])
           || SYMBOLP (attrs[LFACE_OVERLINE_INDEX])
           || STRINGP (attrs[LFACE_OVERLINE_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_STRIKE_THROUGH_INDEX])
+          || INHERITP (attrs[LFACE_STRIKE_THROUGH_INDEX])
           || SYMBOLP (attrs[LFACE_STRIKE_THROUGH_INDEX])
           || STRINGP (attrs[LFACE_STRIKE_THROUGH_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_BOX_INDEX])
+          || INHERITP (attrs[LFACE_BOX_INDEX])
           || SYMBOLP (attrs[LFACE_BOX_INDEX])
           || STRINGP (attrs[LFACE_BOX_INDEX])
           || INTEGERP (attrs[LFACE_BOX_INDEX])
           || CONSP (attrs[LFACE_BOX_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_INVERSE_INDEX])
+          || INHERITP (attrs[LFACE_INVERSE_INDEX])
           || SYMBOLP (attrs[LFACE_INVERSE_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_FOREGROUND_INDEX])
+          || INHERITP (attrs[LFACE_FOREGROUND_INDEX])
           || STRINGP (attrs[LFACE_FOREGROUND_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_BACKGROUND_INDEX])
+          || INHERITP (attrs[LFACE_BACKGROUND_INDEX])
           || STRINGP (attrs[LFACE_BACKGROUND_INDEX]));
    xassert (UNSPECIFIEDP (attrs[LFACE_INHERIT_INDEX])
+          || INHERITP (attrs[LFACE_INHERIT_INDEX])
           || NILP (attrs[LFACE_INHERIT_INDEX])
           || SYMBOLP (attrs[LFACE_INHERIT_INDEX])
           || CONSP (attrs[LFACE_INHERIT_INDEX]));
  #ifdef HAVE_WINDOW_SYSTEM
    xassert (UNSPECIFIEDP (attrs[LFACE_STIPPLE_INDEX])
+          || INHERITP (attrs[LFACE_STIPPLE_INDEX])
           || SYMBOLP (attrs[LFACE_STIPPLE_INDEX])
           || !NILP (Fbitmap_spec_p (attrs[LFACE_STIPPLE_INDEX])));
    xassert (UNSPECIFIEDP (attrs[LFACE_FONT_INDEX])
+          || INHERITP (attrs[LFACE_FONT_INDEX])
           || NILP (attrs[LFACE_FONT_INDEX])
           || STRINGP (attrs[LFACE_FONT_INDEX]));
  #endif
***************
*** 3340,3346 ****
    for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
      if (i != LFACE_FONT_INDEX && i != LFACE_INHERIT_INDEX
        && i != LFACE_AVGWIDTH_INDEX)
!       if (UNSPECIFIEDP (attrs[i])
  #ifdef MAC_OS
          /* MAC_TODO: No stipple support on Mac OS yet, this index is
             always unspecified.  */
--- 3361,3367 ----
    for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
      if (i != LFACE_FONT_INDEX && i != LFACE_INHERIT_INDEX
        && i != LFACE_AVGWIDTH_INDEX)
!       if ((UNSPECIFIEDP (attrs[i]) || INHERITP (attrs[i]))
  #ifdef MAC_OS
          /* MAC_TODO: No stipple support on Mac OS yet, this index is
             always unspecified.  */
***************
*** 3555,3560 ****
--- 3576,3583 ----
        {
        if (i == LFACE_HEIGHT_INDEX && !INTEGERP (from[i]))
          to[i] = merge_face_heights (from[i], to[i], to[i]);
+       else if (INHERITP (from[i]))
+         to[i] = Qunspecified;
        else
          to[i] = from[i];
        }
***************
*** 4047,4053 ****
  
    if (EQ (attr, QCfamily))
      {
!       if (!UNSPECIFIEDP (value))
        {
          CHECK_STRING (value);
          if (SCHARS (value) == 0)
--- 4070,4076 ----
  
    if (EQ (attr, QCfamily))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          CHECK_STRING (value);
          if (SCHARS (value) == 0)
***************
*** 4059,4065 ****
      }
    else if (EQ (attr, QCheight))
      {
!       if (!UNSPECIFIEDP (value))
        {
          Lisp_Object test;
  
--- 4082,4088 ----
      }
    else if (EQ (attr, QCheight))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          Lisp_Object test;
  
***************
*** 4080,4086 ****
      }
    else if (EQ (attr, QCweight))
      {
!       if (!UNSPECIFIEDP (value))
        {
          CHECK_SYMBOL (value);
          if (face_numeric_weight (value) < 0)
--- 4103,4109 ----
      }
    else if (EQ (attr, QCweight))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          CHECK_SYMBOL (value);
          if (face_numeric_weight (value) < 0)
***************
*** 4092,4098 ****
      }
    else if (EQ (attr, QCslant))
      {
!       if (!UNSPECIFIEDP (value))
        {
          CHECK_SYMBOL (value);
          if (face_numeric_slant (value) < 0)
--- 4115,4121 ----
      }
    else if (EQ (attr, QCslant))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          CHECK_SYMBOL (value);
          if (face_numeric_slant (value) < 0)
***************
*** 4104,4110 ****
      }
    else if (EQ (attr, QCunderline))
      {
!       if (!UNSPECIFIEDP (value))
        if ((SYMBOLP (value)
             && !EQ (value, Qt)
             && !EQ (value, Qnil))
--- 4127,4133 ----
      }
    else if (EQ (attr, QCunderline))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        if ((SYMBOLP (value)
             && !EQ (value, Qt)
             && !EQ (value, Qnil))
***************
*** 4118,4124 ****
      }
    else if (EQ (attr, QCoverline))
      {
!       if (!UNSPECIFIEDP (value))
        if ((SYMBOLP (value)
             && !EQ (value, Qt)
             && !EQ (value, Qnil))
--- 4141,4147 ----
      }
    else if (EQ (attr, QCoverline))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        if ((SYMBOLP (value)
             && !EQ (value, Qt)
             && !EQ (value, Qnil))
***************
*** 4132,4138 ****
      }
    else if (EQ (attr, QCstrike_through))
      {
!       if (!UNSPECIFIEDP (value))
        if ((SYMBOLP (value)
             && !EQ (value, Qt)
             && !EQ (value, Qnil))
--- 4155,4161 ----
      }
    else if (EQ (attr, QCstrike_through))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        if ((SYMBOLP (value)
             && !EQ (value, Qt)
             && !EQ (value, Qnil))
***************
*** 4153,4159 ****
        if (EQ (value, Qt))
        value = make_number (1);
  
!       if (UNSPECIFIEDP (value))
        valid_p = 1;
        else if (NILP (value))
        valid_p = 1;
--- 4176,4182 ----
        if (EQ (value, Qt))
        value = make_number (1);
  
!       if (UNSPECIFIEDP (value) || INHERITP (value))
        valid_p = 1;
        else if (NILP (value))
        valid_p = 1;
***************
*** 4210,4216 ****
    else if (EQ (attr, QCinverse_video)
           || EQ (attr, QCreverse_video))
      {
!       if (!UNSPECIFIEDP (value))
        {
          CHECK_SYMBOL (value);
          if (!EQ (value, Qt) && !NILP (value))
--- 4233,4239 ----
    else if (EQ (attr, QCinverse_video)
           || EQ (attr, QCreverse_video))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          CHECK_SYMBOL (value);
          if (!EQ (value, Qt) && !NILP (value))
***************
*** 4221,4227 ****
      }
    else if (EQ (attr, QCforeground))
      {
!       if (!UNSPECIFIEDP (value))
        {
          /* Don't check for valid color names here because it depends
             on the frame (display) whether the color will be valid
--- 4244,4250 ----
      }
    else if (EQ (attr, QCforeground))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          /* Don't check for valid color names here because it depends
             on the frame (display) whether the color will be valid
***************
*** 4235,4241 ****
      }
    else if (EQ (attr, QCbackground))
      {
!       if (!UNSPECIFIEDP (value))
        {
          /* Don't check for valid color names here because it depends
             on the frame (display) whether the color will be valid
--- 4258,4264 ----
      }
    else if (EQ (attr, QCbackground))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          /* Don't check for valid color names here because it depends
             on the frame (display) whether the color will be valid
***************
*** 4250,4256 ****
    else if (EQ (attr, QCstipple))
      {
  #ifdef HAVE_X_WINDOWS
!       if (!UNSPECIFIEDP (value)
          && !NILP (value)
          && NILP (Fbitmap_spec_p (value)))
        signal_error ("Invalid stipple attribute", value);
--- 4273,4279 ----
    else if (EQ (attr, QCstipple))
      {
  #ifdef HAVE_X_WINDOWS
!       if (!UNSPECIFIEDP (value) && !INHERITP (value)
          && !NILP (value)
          && NILP (Fbitmap_spec_p (value)))
        signal_error ("Invalid stipple attribute", value);
***************
*** 4260,4266 ****
      }
    else if (EQ (attr, QCwidth))
      {
!       if (!UNSPECIFIEDP (value))
        {
          CHECK_SYMBOL (value);
          if (face_numeric_swidth (value) < 0)
--- 4283,4289 ----
      }
    else if (EQ (attr, QCwidth))
      {
!       if (!UNSPECIFIEDP (value) && !INHERITP (value))
        {
          CHECK_SYMBOL (value);
          if (face_numeric_swidth (value) < 0)
***************
*** 4285,4291 ****
          else
            f = check_x_frame (frame);
  
!         if (!UNSPECIFIEDP (value))
            {
              CHECK_STRING (value);
  
--- 4308,4314 ----
          else
            f = check_x_frame (frame);
  
!         if (!UNSPECIFIEDP (value) && !INHERITP (value))
            {
              CHECK_STRING (value);
  
***************
*** 4333,4339 ****
      signal_error ("Invalid face attribute name", attr);
  
    if (font_related_attr_p
!       && !UNSPECIFIEDP (value))
      /* If a font-related attribute other than QCfont is specified, the
         original `font' attribute nor that of default face is useless
         to determine a new font.  Thus, we set it to nil so that font
--- 4356,4362 ----
      signal_error ("Invalid face attribute name", attr);
  
    if (font_related_attr_p
!       && !UNSPECIFIEDP (value) && !INHERITP (value))
      /* If a font-related attribute other than QCfont is specified, the
         original `font' attribute nor that of default face is useless
         to determine a new font.  Thus, we set it to nil so that font
***************
*** 4354,4360 ****
        ++windows_or_buffers_changed;
      }
  
!   if (!UNSPECIFIEDP (value)
        && NILP (Fequal (old_value, value)))
      {
        Lisp_Object param;
--- 4377,4383 ----
        ++windows_or_buffers_changed;
      }
  
!   if (!UNSPECIFIEDP (value) && !INHERITP (value)
        && NILP (Fequal (old_value, value)))
      {
        Lisp_Object param;
***************
*** 8012,8017 ****
--- 8035,8042 ----
    staticpro (&Qforeground_color);
    Qunspecified = intern ("unspecified");
    staticpro (&Qunspecified);
+   Qinherit = intern ("inherit");
+   staticpro (&Qinherit);
  
    Qface_alias = intern ("face-alias");
    staticpro (&Qface_alias);

Diff finished.  Thu Nov 10 22:09:15 2005