From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Question about display engine Date: Sun, 29 Sep 2019 13:57:22 +0300 Message-ID: <83impbfjzh.fsf@gnu.org> References: <20190908182346.hheaveun2pw5usb6@Ergus> <20190914204207.gfyvgbb7t4ztya7a@Ergus> <83ftkxy3r7.fsf@gnu.org> <20190915214233.xkjtoxyfxkyrd2id@Ergus> <20190917021725.xxhhhxcz3nr6sb7z@Ergus> <83blvjw8x9.fsf@gnu.org> <83v9tmqcv7.fsf@gnu.org> <20190921215551.ruu6ji6sjpxydpng@Ergus> <20190926163204.gdb4gxbjtdbysk3y@Ergus> <83zhiohfnx.fsf@gnu.org> <20190929103034.jw5qolfjtbjkbk5p@Ergus> Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="162116"; mail-complaints-to="usenet@blaine.gmane.org" Cc: rudalics@gmx.at, emacs-devel@gnu.org To: Ergus Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Sep 29 12:58:03 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1iEWtu-000g6N-Qk for ged-emacs-devel@m.gmane.org; Sun, 29 Sep 2019 12:58:02 +0200 Original-Received: from localhost ([::1]:37898 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iEWtt-0005nK-Kl for ged-emacs-devel@m.gmane.org; Sun, 29 Sep 2019 06:58:01 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:43754) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iEWtN-0005mz-Ug for emacs-devel@gnu.org; Sun, 29 Sep 2019 06:57:30 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:46587) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1iEWtN-0005EX-6S; Sun, 29 Sep 2019 06:57:29 -0400 Original-Received: from [176.228.60.248] (port=3930 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1iEWtM-0002pn-Cl; Sun, 29 Sep 2019 06:57:28 -0400 In-reply-to: <20190929103034.jw5qolfjtbjkbk5p@Ergus> (message from Ergus on Sun, 29 Sep 2019 12:30:34 +0200) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:240387 Archived-At: > Date: Sun, 29 Sep 2019 12:30:34 +0200 > From: Ergus > Cc: rudalics@gmx.at, emacs-devel@gnu.org > > > . I don't understand why you changed the underlined_p and > > underline_type stuff in struct face. What were the reasons for > > that part of the changeset? Its sole effect seems to be to > > generate some redundant changes, or am I missing something? > > > Use double variable to describe the same is an error prone > practice. This change simplifies the checks in all the code related (as > there will be only one variable to set/check), reduces one name and > member in the struct and avoids errors related to changing one value and > not the other. That's your stylistic preference, and I can understand it. But when the only reason for a change is stylistic preferences, I generally prefer to leave the code intact, so that the original authors' version remains as long as it does TRT. > > . In this hunk from append_space_for_newline, why did you change > > FACE_FROM_ID_OR_NULL to FACE_FROM_ID? > > > > - int local_default_face_id = > > + int char_width = 1; > > + > > + if (default_face_p > > +#ifdef HAVE_WINDOW_SYSTEM > > + || FRAME_WINDOW_P (it->f) > > +#endif > > + ) > > + { > > + const int local_default_face_id = > > lookup_basic_face (it->w, it->f, DEFAULT_FACE_ID); > > struct face* default_face = > > - FACE_FROM_ID_OR_NULL (it->f, local_default_face_id); > > + FACE_FROM_ID (it->f, local_default_face_id); > > > > The call is made after a lookup_basic_face and it's for DEFAULT_FACE_ID > is it needed the check? Normally for other faces if it is NULL then we > use the DEFAULT_FACE_ID. In this case it should be unneeded right? It's the other way around: FACE_FROM_ID could trigger an assertion violation, where FACE_FROM_ID_OR_NULL will silently return a NULL pointer. So we should only use FACE_FROM_ID where we are 110% certain it can never violate the assertion, or where a NULL pointer for a face will cause worse problems than an assertion. Thanks.