From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#11068: 24.0.94; Face-remapped background does not extend to end of window Date: Sun, 25 Mar 2012 06:02:49 +0200 Message-ID: <83ty1ds4ye.fsf@gnu.org> References: <1BE52A40-0403-433F-8164-DFDBD6771F80@gmail.com> <83ty1fvc28.fsf@gnu.org> <83obrmtbsy.fsf@gnu.org> <87ty1ekrqy.fsf@gnu.org> <83ehsit63e.fsf@gnu.org> <87mx75z8n6.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: dough.gmane.org 1332648253 6262 80.91.229.3 (25 Mar 2012 04:04:13 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sun, 25 Mar 2012 04:04:13 +0000 (UTC) Cc: darthandrus@gmail.com, 11068@debbugs.gnu.org To: Chong Yidong Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Mar 25 06:04:11 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1SBegY-0008Bo-4O for geb-bug-gnu-emacs@m.gmane.org; Sun, 25 Mar 2012 06:04:06 +0200 Original-Received: from localhost ([::1]:54418 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SBegX-0000jL-8z for geb-bug-gnu-emacs@m.gmane.org; Sun, 25 Mar 2012 00:04:05 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:42239) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SBegU-0000j1-B8 for bug-gnu-emacs@gnu.org; Sun, 25 Mar 2012 00:04:03 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SBegS-0002MA-EF for bug-gnu-emacs@gnu.org; Sun, 25 Mar 2012 00:04:01 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:58782) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SBegS-0002M5-9J for bug-gnu-emacs@gnu.org; Sun, 25 Mar 2012 00:04:00 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1SBfAT-0003Xo-KK for bug-gnu-emacs@gnu.org; Sun, 25 Mar 2012 00:35:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 25 Mar 2012 04:35:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 11068 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 11068-submit@debbugs.gnu.org id=B11068.133265006713583 (code B ref 11068); Sun, 25 Mar 2012 04:35:01 +0000 Original-Received: (at 11068) by debbugs.gnu.org; 25 Mar 2012 04:34:27 +0000 Original-Received: from localhost ([127.0.0.1]:37381 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SBf9u-0003X1-Mk for submit@debbugs.gnu.org; Sun, 25 Mar 2012 00:34:27 -0400 Original-Received: from mtaout22.012.net.il ([80.179.55.172]:48202) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SBf9N-0003Vy-7O for 11068@debbugs.gnu.org; Sun, 25 Mar 2012 00:34:26 -0400 Original-Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0M1F00M009S29500@a-mtaout22.012.net.il> for 11068@debbugs.gnu.org; Sun, 25 Mar 2012 06:02:50 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([84.229.240.24]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0M1F00L2Y9WPQAF0@a-mtaout22.012.net.il>; Sun, 25 Mar 2012 06:02:50 +0200 (IST) In-reply-to: <87mx75z8n6.fsf@gnu.org> X-012-Sender: halo1@inter.net.il X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:58115 Archived-At: > From: Chong Yidong > Cc: darthandrus@gmail.com, monnier@iro.umontreal.ca, 11068@debbugs.gnu.org > Date: Sun, 25 Mar 2012 11:01:17 +0800 > > Eli Zaretskii 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. OK. > 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. Right. > 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? Here I disagree: I think we should confine the internal details of face remapping to lookup_basic_face, instead of spilling our knowledge about that all over. As you just said above, if face-remapping-alist is nil, that function returns immediately, so there's nothing lost here for the "usual" use-case. > 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. OK, but to be absolutely fair, I must point out that, while the patch is very simple, it has one non-trivial consequence: when the default face _is_ remapped, the empty space to the right of any L2R line and the empty lines beyond EOB are filled with stretch glyphs of a suitably computed width. Before the patch, there were no glyphs in those places. So, from the POV of the content of the glyph matrices, the change introduced by these patches is quite prominent. In particular, this could potentially affect any code that examines the glyphs of a glyph matrix; previously, such stretch glyphs were only present at the left side of R2L glyph rows.