[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#11068: 24.0.94; Face-remapped background does not extend to end of w

From: Chong Yidong
Subject: bug#11068: 24.0.94; Face-remapped background does not extend to end of window
Date: Sun, 25 Mar 2012 11:01:17 +0800
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.94 (gnu/linux)

Eli Zaretskii <address@hidden> writes:

>> This looks a bit strange.  Why is DEFAULT_FACE_ID handled specially
>> here?
> In my testing, I didn't see the need to do it even for the default
> face, because it->face_id is already set to the ID of the remapped
> face.  So it's just me being paranoiac.

I'd just leave that bit out.

Also, in this hunk

@@ -18173,7 +18173,12 @@
          it->len = 1;
          if (default_face_p)
-           it->face_id = DEFAULT_FACE_ID;
+           {
+             if (NILP (Vface_remapping_alist))
+               it->face_id = DEFAULT_FACE_ID;
+             else
+               it->face_id = lookup_basic_face (it->f, DEFAULT_FACE_ID);
+           }
          else if (it->face_before_selective_p)
            it->face_id = it->saved_face_id;
          face = FACE_FROM_ID (it->f, it->face_id);

You should call lookup_basic_face without the surrounding if statement,
because lookup_basic_face returns its second arg immediately if
face-remapping-alist is nil, making the extra check redundant.

And here

+         if (row->reversed_p
+             || lookup_basic_face (it->f, DEFAULT_FACE_ID) != DEFAULT_FACE_ID)

shouldn't you just compare default_face->id to DEFAULT_FACE_ID instead
of making another lookup_basic_face call?

With these changes, the patch seems pretty safe; even if something
screws up, it will only affect how remapped faces extend to the end of
the buffer, which was broken before anyway.  So feel free to commit.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]