From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Ergus Newsgroups: gmane.emacs.devel Subject: Re: Question about display engine Date: Mon, 7 Oct 2019 17:40:55 +0200 Message-ID: <20191007154054.zfbsuktkaue5ymbr@Ergus> References: <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> <83impbfjzh.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="125866"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: NeoMutt/20180716 Cc: rudalics@gmx.at, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Oct 07 17:54:48 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 1iHVLS-000Waz-M2 for ged-emacs-devel@m.gmane.org; Mon, 07 Oct 2019 17:54:46 +0200 Original-Received: from localhost ([::1]:46594 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iHVLR-0000KK-81 for ged-emacs-devel@m.gmane.org; Mon, 07 Oct 2019 11:54:45 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:50628) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iHV8S-0004vs-Eg for emacs-devel@gnu.org; Mon, 07 Oct 2019 11:41:22 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iHV8P-00029L-P3 for emacs-devel@gnu.org; Mon, 07 Oct 2019 11:41:19 -0400 Original-Received: from sonic307-54.consmr.mail.ir2.yahoo.com ([87.248.110.31]:44427) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iHV8N-00027j-TW for emacs-devel@gnu.org; Mon, 07 Oct 2019 11:41:17 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aol.com; s=a2048; t=1570462872; bh=jSnbw2YUXV2otLO0FAU0b33lmUzAsapQjfUqzuvucTk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From:Subject; b=tdXTMH6zTtVPCNfLIqccRASXuB3BBxD2YKuA9S+2WuxRxGZwn3EZBNEC8yZilK7cajWakLjcJ0QXZ3uojDX7GwcNFG7mhr3gJQW8dehEmcN671gWvZVqd7dLwMG9eLJgSfdpuuLygQsdTgnqe52G7RLiE8kBvFaH8jcPpKhFoKeaf9R/WZxUpsJDR7nlcjYotv1kF1lmPBzqo+fwUIcgKd7UeZAln7zoSWrdV7SNP3qj5566RaZFgNDSAVttL1650obAu1GmqPw+25NBoXImUNBxRmW+94ORml/n8qOC222VDp9Perb3813f2www0I+PbDQn96jC0iSBFrfiac/LVw== X-YMail-OSG: poudOlAVM1mXIcWoT7qQ35CI4TfYaFX6dB5MT2reIzFl12xIV6O2lkdymCOe8lX kDrwLolcYAoKYQCna0850LWSg5BqR602L8WXLGDYMsYcxZAVL5ubYfCEmqRw9srOcSHe5R21J9pU ncbY9ODWil4MdEhHzSozkZF1nPgjqZgiF6Iiavm_4ukQz0qdMDuUeISeiY45xT37sV_F91px7oOr 2P4WuDajlDH8Oz21Z0TV.w2l50r9tlFSLjCP863MeK1CdzgiwiJUfv1HRnJnEGR3S4jctFWxwAdx Bi0MKCv6BWSXQ9AyhaL8p9IWHA7gzMfN4vqoaDeDqhQfMJCkLwWOs7I8ATTcGBQTe7R_cbblld6x yABY26ROtvOB6n38Kao6hKzO89dmOJnmeaA8qys7EKvYFaRssT_7YQHDza76tc43_T5Y80WN6hYg EBcLY11jdEjQykEeCLW8y75OdYV9jqKXdseueHbEjrW7pMSr4P._Au2l.Ya_BLD0LVsQYkdJsfPW qxZY70ukXJc_xGmKDJTaTsUtqtdoWOVh6M9OCMsZqAirCCqog.gmfcCCbb35Yy_7h9CLi4.jn.OY 3o4wGUJHSckKaR_B_mM5yk4J_ckQbfs1sBGmhwNBCWJL0RzlX3n.GB1k5X1i77slCeryqkI_cNQn YlaLnLJRvjh8QGj6KKYBdPdFUqGTfQ8wF1CnCwHx1xWdjRrGyfipaXvFZcymgdrJzguFB3qqKMxE .wPz3ctwJ1Lwe.cB_cBQEez4O9QSiyDrYCHKJGmi5_n8zxMX35Lh8QrgnS9tEVY37iAEK2pfQ0cd Yw7m__JQJmtdTa6RSmY30I7z2qoK9azKO8_o2Jkzhq Original-Received: from sonic.gate.mail.ne1.yahoo.com by sonic307.consmr.mail.ir2.yahoo.com with HTTP; Mon, 7 Oct 2019 15:41:12 +0000 Original-Received: by smtp430.mail.ir2.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID 258284e8e55fd4c1c13e45701a6a0812; Mon, 07 Oct 2019 15:41:07 +0000 (UTC) Content-Disposition: inline In-Reply-To: <83impbfjzh.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 87.248.110.31 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:240695 Archived-At: Hi Eli: I made some of the fixes you suggested in the last email about this topic. The reduction of the underline-p + underline-stile into a single underline variable in the struct for me still seems like a good change for readability and simplification in all the code. Any way in the future I will try to avoid this kind of modifications even when they seems to be an improvement... but could you let it pass this time? The other changes that seemed to be stylistic too, were actually in portions of code I wrote for dfci and It was there because I copied some of it from the previous if-else section... but I can correct it now right?... so no problem? If this is fine may I merge in master? Best, Ergus. On Sun, Sep 29, 2019 at 01:57:22PM +0300, Eli Zaretskii wrote: >> 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. >