From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages Date: Mon, 2 Dec 2019 02:05:10 +0200 Message-ID: References: <87o8xwrjba.fsf@bernoul.li> <834kzooo8e.fsf@gnu.org> <877e4d7yzf.fsf@bernoul.li> <83imnvg53q.fsf@gnu.org> <87zhh2ofc9.fsf@bernoul.li> <87k186nsku.fsf@bernoul.li> <87imna18nc.fsf@mail.linkov.net> <42c596c2-b5c1-9fc9-4b92-9c13b386d93d@yandex.ru> <83pnhgrlni.fsf@gnu.org> <83ftiasfdm.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="19239"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 Cc: 37774@debbugs.gnu.org, juri@linkov.net To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Dec 02 01:06:17 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1ibZEF-0004s2-Ob for geb-bug-gnu-emacs@m.gmane.org; Mon, 02 Dec 2019 01:06:15 +0100 Original-Received: from localhost ([::1]:56982 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ibZED-0002Yq-Vo for geb-bug-gnu-emacs@m.gmane.org; Sun, 01 Dec 2019 19:06:14 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:42427) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ibZE3-0002US-CM for bug-gnu-emacs@gnu.org; Sun, 01 Dec 2019 19:06:04 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ibZE2-0002ZZ-00 for bug-gnu-emacs@gnu.org; Sun, 01 Dec 2019 19:06:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:59131) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ibZE1-0002ZV-SR for bug-gnu-emacs@gnu.org; Sun, 01 Dec 2019 19:06:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ibZE1-0004AT-N6 for bug-gnu-emacs@gnu.org; Sun, 01 Dec 2019 19:06:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 02 Dec 2019 00:06:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 37774 X-GNU-PR-Package: emacs Original-Received: via spool by 37774-submit@debbugs.gnu.org id=B37774.157524512215963 (code B ref 37774); Mon, 02 Dec 2019 00:06:01 +0000 Original-Received: (at 37774) by debbugs.gnu.org; 2 Dec 2019 00:05:22 +0000 Original-Received: from localhost ([127.0.0.1]:36871 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ibZDN-00049O-Kr for submit@debbugs.gnu.org; Sun, 01 Dec 2019 19:05:22 -0500 Original-Received: from mail-wm1-f51.google.com ([209.85.128.51]:51466) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ibZDL-000497-R7 for 37774@debbugs.gnu.org; Sun, 01 Dec 2019 19:05:20 -0500 Original-Received: by mail-wm1-f51.google.com with SMTP id g206so19878940wme.1 for <37774@debbugs.gnu.org>; Sun, 01 Dec 2019 16:05:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=yeSz7iI8oLzfB6iDZEuZAuU7q9td5U9W3i0hEIs+mdM=; b=l29cHctQ9gaku2DbO+2Omm4oztIsKaVJ3Wc/F8RPWA8A0wC/iHLO5bhDRB2QhuyAf6 5WD2Te01LeyrFCQ3G6kdkOJjah7iXjSctAXsEBdUEh976Hjoo2ZvcTKgmI+y7yIla4KK icIZQhFvFJvc0v1k4++DWYMacfriZzpJkxS9G8b0/D+/5BA6KEg6eBNivSHMi5Po7dJi Z0teK8Q7DJDqX8TM/ZZalxYDBs8nJ0/hAnL9i9gRR+efu6B2oEsnyPSrGu6IjzXB+juZ x8dSePMI9Zk0ECONBL192Zme/0nY3tID19s6QpT7Lb74V2a0yZwEbzatryMjzMVp/c8q IxDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:cc:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=yeSz7iI8oLzfB6iDZEuZAuU7q9td5U9W3i0hEIs+mdM=; b=lcLuZzFp23pBBaFFRD3pc4mlNQanAGDDlqZvBxURRYjCtG9F1B7i6Mq1dptGQfuPHf e1hOQabWJt9dLJQgS+7Jr05K/jgQyUJVlD7yAjddhg7HCRW/NgqQMhJoF8UhZ4MhovUa U831dEZ9OmQvlAHlJuNw5I4aakvlM43LXv4SdMMw1MhxcuiGrE9QR5wiQaF4LLqIZ2/A jcEJMNFJ4JIix+zt5lgRr3Nz5uSmK13U/edwsYvW7NzNZIlJFeJhPoMEVkuCJmbDPjPB crCbmV7F9U+EaCwBj4PvKBcz5ll3OZHlEjbieNgHTYnoJzCt8/IX/nZYL5erNSnKBh1k MbAw== X-Gm-Message-State: APjAAAV0lt+f35ll+gAseNoAv3SefKUzQCbWsnnL8Tj1uGdbTRq9rdat i0DrbM53Ri7uMaWwMOuPSMg= X-Google-Smtp-Source: APXvYqxxug3u0rfcAJTaZhNhcL/hW+XdjIKA7oHUc33hY/amyWNplzL7znJqpJBrtT6PSV5OqBYx7A== X-Received: by 2002:a1c:8156:: with SMTP id c83mr23903024wmd.59.1575245113859; Sun, 01 Dec 2019 16:05:13 -0800 (PST) Original-Received: from [192.168.0.5] ([212.50.117.215]) by smtp.googlemail.com with ESMTPSA id g184sm23405943wma.8.2019.12.01.16.05.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 01 Dec 2019 16:05:12 -0800 (PST) In-Reply-To: <83ftiasfdm.fsf@gnu.org> Content-Language: en-US X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.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" Xref: news.gmane.org gmane.emacs.bugs:172741 Archived-At: On 26.11.2019 19:43, Eli Zaretskii wrote: >> It's not for context diffs, it's for context around the changes in >> unified diffs as well. Notice the gray background on the screenshot. > > Ah, okay. Still, feel free to change its :extend attribute. OK, done. But it feels pretty useless, considering any theme where this would affect something would have to repeat the 'extend t' definition anyway. And this would work without the change I just made anyway. Too bad we don't have a way to affect all themes at once here. >>> We need to modify all the themes we provide to specify :extend for >>> faces where we do that by default. It seems there's no way around >>> that, since the semantics of custom-theme-set-faces is clearly to >>> reset all face attributes to 'unspecified' before applying the face >>> spec, so keeping some attributes from the default face spec is out of >>> the question, unfortunately. It's clear that the faces stuff was not >>> designed to accommodate addition of attributes easily. >> >> Seems like it's a consequence of the implementation strategy. There were >> a couple of others that had been proposed (splitting the attribute in >> two, with different default values) or using a symbol property. > > Splitting into two attributes won't help here (quite the contrary, > since we'd have to deal with 2 new attributes). That depends on the end goal. If we were aiming to keep the previous behavior almost entirely, but avoid extending things like underline over newlines, there are two default values for the two proposed attributes that would satisfy almost everybody. And the people who would prefer not to extend the region face background over newlines would apply that in their init scripts (ditto for other faces). Sounds like a win-win, avoiding the necessity to update all themes, first- and third-party ones. Also it seems like it's close to the way we usually introduce far-reaching changes in Emacs. But it's one option. > As for the symbol > property suggestion, see below. See below as well (option two, in my opinion). >> You said the latter would complicate the face merging code (which makes >> sense) and "is extremely unclean". I have to take you at your word here. > > You don't have to take my word for it, the code is there to read and > make up your own mind. Forgive my ignorance, but xfaces.c is 7K lines long. It will be easier to speak high-level, but please correct whatever misconceptions I may bring. > Basically, on the C level each Lisp face is represented by an array of > its attributes, where each array element holds the value of the > corresponding attribute. Once this array is computed, we can (and do) > manipulate just the array, and for all practical purposes can forget > about the face's symbol. Using a symbol property would then need to > keep the symbol around at all times, which is inconvenient and would > make the code ugly. I don't think so. Once the symbol is gone, whatever left is just the value. So when this array is computed, the function doing that would merge the faces attributes with whatever attributes are specified using the alternative symbol property. To be more exact, the current face attributes are also assigned to a symbol property (face-defface-spec). We can add another property: face-default-spec, which would contain attributes that should apply to the face unless explicitly overridden in face-defface-spec. It could even be set by a new defface keyword instead of plain 'put', but that's a minor concern IMO. (Option two ends here). This seems to be the easiest way to go around the long-established behavior that custom-theme-set-faces overwrites the face attributes (instead of trying to merge them). We could do in the other way, but it would require changes in both custom-theme-set-faces and defface, as well as some other functions I imagine, and either a whitelist of attributes that would always be retained unless overridden. Or a wholesale change to retain all attributes by default. I might like the last option personally, but it would be a major breaking change. Still, all themes could be updated to account for it while keeping compatibility with older Emacs with little trouble (which is harder to do with the current :extend situation). > But even if we'd overcome this annoyance, how do you specify this > property for a face like below? > > '(:inherit foo :background "green" :underline "red") > > There's no symbol to put the property on. Do we say that such > anonymous faces cannot support this attribute? Unclean. See above. Just add ':extend t' (or nil) to the end of the value.