From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#59347: 29.0.50; `:family` face setting ignored Date: Mon, 12 Dec 2022 17:07:07 +0200 Message-ID: <831qp4slt0.fsf@gnu.org> References: <83zgcl4jra.fsf@gnu.org> <7cc9e03786c281cffdd4@heytings.org> <83tu2t4ie9.fsf@gnu.org> <7cc9e03786e324ff82ef@heytings.org> <83bkp04gjl.fsf@gnu.org> <83leo42vm9.fsf@gnu.org> <0d1ea3007fd94b7ae0b1@heytings.org> <83r0xv1649.fsf@gnu.org> <0d1ea3007f532a493429@heytings.org> <83cz9f12bh.fsf@gnu.org> <835yewleyn.fsf@gnu.org> <83tu2b9rlx.fsf@gnu.org> <83k0347gtu.fsf@gnu.org> <83v8mm2ug7.fsf@gnu.org> <83cz8u2d6u.fsf@gnu.org> <831qp93nsc.fsf@gnu.org> <1a7e3acf3578520feda7@heytings.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="17114"; mail-complaints-to="usenet@ciao.gmane.io" Cc: luangruo@yahoo.com, monnier@iro.umontreal.ca, 59347@debbugs.gnu.org To: Gregory Heytings Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Dec 12 16:08:21 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1p4kPl-0004Fd-3e for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 12 Dec 2022 16:08:21 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p4kPU-0001lh-BV; Mon, 12 Dec 2022 10:08:04 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p4kPS-0001l5-IC for bug-gnu-emacs@gnu.org; Mon, 12 Dec 2022 10:08:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1p4kPS-0005wB-AH for bug-gnu-emacs@gnu.org; Mon, 12 Dec 2022 10:08:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1p4kPR-00080Z-Pa for bug-gnu-emacs@gnu.org; Mon, 12 Dec 2022 10:08:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 12 Dec 2022 15:08:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 59347 X-GNU-PR-Package: emacs Original-Received: via spool by 59347-submit@debbugs.gnu.org id=B59347.167085763630772 (code B ref 59347); Mon, 12 Dec 2022 15:08:01 +0000 Original-Received: (at 59347) by debbugs.gnu.org; 12 Dec 2022 15:07:16 +0000 Original-Received: from localhost ([127.0.0.1]:53537 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p4kOh-00080G-VD for submit@debbugs.gnu.org; Mon, 12 Dec 2022 10:07:16 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:43440) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p4kOg-000809-7l for 59347@debbugs.gnu.org; Mon, 12 Dec 2022 10:07:14 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p4kOa-0005oG-UB; Mon, 12 Dec 2022 10:07:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=uNYhxDWeX+N/NaVX8DGXoPKZs4Eb9YnLzjy5B2NAwBA=; b=GUlfIF0LQprR JZKt46HAV5Byiy4ysVIoY9lclzo7XxDu5H0HjRNgoDtdzPIRGZwfr4xMi2tsMOd+jP/o8thfwWIgy VbxCvCbrrbs+A1eQ32XzJ1QofkbPoGG3oCDZXiVwEkb0o9fF9ytJ6LLCOS8Cilg3W6CY8n5Jsi7Si GWGKAl1+ynzJH74L4pJ9RCekz9PIb+ylGYwOpWYNo+DfB9dFgbVcMDAqxUDCxgb/1EDwYx+VHDldU TA20QxlxkOiX+m2XUfcOC/8ff5Brzgj4TKTU3tdwqyhUY+iVpa/ehg4DUUTmpmjP/5+noKsd/zVdY c37h29kJMoUA3q5x23f8zQ==; Original-Received: from [87.69.77.57] (helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p4kOY-00009i-6I; Mon, 12 Dec 2022 10:07:08 -0500 In-Reply-To: (message from Gregory Heytings on Mon, 12 Dec 2022 00:57:49 +0000) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:250710 Archived-At: > From: Gregory Heytings > cc: monnier@iro.umontreal.ca, Po Lu , > 59347@debbugs.gnu.org > > > Amazing. I thought this exhausting discussion was over, but no, Po Lu > came and "improved" the code that was agreed upon only a couple of hours > after it was pushed, disregarding this entire discussion, the docstring of > the variable that the commit introduced, the commit message, and without > asking any questions. How can that be right, how can that be acceptable? This discussion went overboard, IMO. I indeed think that Po Lu should have discussed his changes before doing them, but your reaction, in particular the revert, is also overreaction. > The name of the variable was changed, and the docstring was "improved", > and became completely wrong. It is simply untrue that this is a list of > attributes that Emacs will "ignore when realizing a face", or in fact that > this changes anything fundamental in the way Emacs realizes faces. If this is the doc string you saw, you were not looking at an up-to-date tree. I renamed the variable and rewrote the doc string soon after Po Lu made his changes; the word "ignore" (which I agree is inaccurate) is no longer there. If you still have comments on the current doc string, please speak up. > Here is again, in every possible detail, but this time in a single post, > the analysis of this subtle bug, and the rationale of the patch that was > agreed upon. I explain this bug with an concrete example, with the > 'variable-pitch' face and a font with a 'medium' weight. That example is > only meant to illustrate the bug: the same reasoning applies for other > faces and other font attributes (slant or width). Thanks. This is a good description, and it is good to have it in the bug discussion, for posterity. > From: Po Lu > Cc: Gregory Heytings > > - unsetting the "extra" attribute is not safe on the Haiku port. If this is so, why not disable that only for Haiku? > - the bitmask variable is a real nusiance for anyone trying to > debug Emacs or change the layout of the font attribute index > enumerator. "Nuisance" is an exaggeration. But I agree that using more descriptive values is more convenient, if and when someone needs to change the default value. And I have a proposal for how to do this without sacrificing performance; read on. > Just because a bug has been closed does NOT mean the change in it is no > longer subject to scrutiny. But, after such a long and painful discussion, it would have been prudent to talk first and cut the metal later. And, btw, your change had two copy/paste typos, which would have probably be avoided if you posted the patch first. > all that code takes somewhere between 2 to 4 microseconds to complete > for me, in a build with optimizations enabled. I don't know how many People are reportedly running Emacs sessions with several thousands of faces, in which case 2 to 4 usec per face could add up to a significant number. So it cannot do any harm to try to make the "usual" case faster. > From: Gregory Heytings > To: Po Lu > cc: emacs-devel@gnu.org > > >> Of course I did. That you did not read it is another thing. > > > > You did not. The only real technical argument you put forth was some > > nonsense about FOR_EACH_TAIL being slow. > > > > "Nonsense", again? Thank you! Yes, let's please avoid nasty unfriendly language. > From: Po Lu > To: Gregory Heytings > Cc: Eli Zaretskii , monnier@iro.umontreal.ca, > 59347@debbugs.gnu.org > > The fundamental problem I have with the doc string is > that it did not say what it does, or what it is supposed to debug, in a > manner users can understand. Does the current doc string have any such problems you can spot? Anyway, here's my proposal: . we change the default value of the variable to be t, and document that this stands for (:weight :width :slant) . we change the code to reset only those 3 attributes when the value is t, and to reset nothing when the value is nil . the (slower) code which loops over the list will only run if the value of the variable is neither nil nor t . we avoid resetting the :extra attribute on Haiku Is this okay with you both? I volunteer to make these changes if you agree. Thanks.