From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Clemens Newsgroups: gmane.emacs.bugs Subject: bug#45780: 28.0.50; [PATCH] Face used for affixation function annotations Date: Mon, 11 Jan 2021 21:07:57 +0100 Message-ID: References: <9b1409af-58b9-3a62-4162-343fd25293ca@posteo.net> <87ft378gjk.fsf@mail.linkov.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28919"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 Cc: 45780@debbugs.gnu.org To: Juri Linkov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Jan 11 21:09:11 2021 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 1kz3V0-0007R5-KJ for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 11 Jan 2021 21:09:10 +0100 Original-Received: from localhost ([::1]:55626 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kz3Uz-0000Rd-Ak for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 11 Jan 2021 15:09:09 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:33754) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kz3Us-0000RL-KU for bug-gnu-emacs@gnu.org; Mon, 11 Jan 2021 15:09:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:46810) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kz3Us-0007Qk-DM for bug-gnu-emacs@gnu.org; Mon, 11 Jan 2021 15:09:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kz3Us-00049U-6t for bug-gnu-emacs@gnu.org; Mon, 11 Jan 2021 15:09:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Clemens Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 11 Jan 2021 20:09:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 45780 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 45780-submit@debbugs.gnu.org id=B45780.161039568715896 (code B ref 45780); Mon, 11 Jan 2021 20:09:02 +0000 Original-Received: (at 45780) by debbugs.gnu.org; 11 Jan 2021 20:08:07 +0000 Original-Received: from localhost ([127.0.0.1]:58356 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kz3Tz-00048K-HK for submit@debbugs.gnu.org; Mon, 11 Jan 2021 15:08:07 -0500 Original-Received: from mout01.posteo.de ([185.67.36.65]:36863) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kz3Tx-00047m-6i for 45780@debbugs.gnu.org; Mon, 11 Jan 2021 15:08:06 -0500 Original-Received: from submission (posteo.de [89.146.220.130]) by mout01.posteo.de (Postfix) with ESMTPS id A6BFF16005C for <45780@debbugs.gnu.org>; Mon, 11 Jan 2021 21:07:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1610395678; bh=ApcUXNLVt09YYprXjh3m141ZP0oz8M1ODyUmtDXXgbw=; h=Subject:To:Cc:From:Date:From; b=GgGx5N9136Xe4hW5iCm1x/crdwLHbEw60gSoQu/4LuabocwkSGAnhJEkq+/shVERA YKi1z4pegyROT4pegOszTMKZGbppGwbL9uXb+6J6GgTb0t5kCtuRRJqAT/qo4TEsaB FaeoT5wG1Xsm2oicMKKChVEMuUMLXR2UO+H7zbgG/dis9cRSv6PEPlm5FCEtRBlx+X H4Cdcf92USFJLsGAzpi7UY/NxbP3AIOHjQZrQno+lqjmFLupyXchIAeI9TvmoR9PdX N2tpO1OXNjNyMAf5P3USATOF7XOO9a/2CqtZA7FUccD/n42mFWhVKWluEJzdYK09Rl 7j5tOWywy+D2g== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4DF4Wf01PQz6tmh; Mon, 11 Jan 2021 21:07:57 +0100 (CET) In-Reply-To: <87ft378gjk.fsf@mail.linkov.net> Content-Language: en-US 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" Xref: news.gmane.io gmane.emacs.bugs:197747 Archived-At: >> As per the comment above the affected code, the client can specify the face >> when prefix and suffix are provided. The prefix is already checked earlier >> and what remained was checking the suffix not the prefix. > > Shouldn't then this code with font-lock-prepend-text-property > be removed completely? Since both prefix and suffix are non-nil, > this makes code no-op. You are right I assumed the suffix can also be nil but looking at the let binding earlier in the code this can't be the case when there is a prefix which is derived from the fact that there is a suffix annotation in the first place :) >> There is another thing I would like to bring up in this context: When the >> annotations returned by annotation/affixation functions already specify >> a face I think it would be nicer if the completion-annotations face >> wouldn't be applied generally. In Selectrum we use something like: >> >> (if (text-property-not-all 0 (length str) 'face nil str) >> str >> (propertize str 'face 'completions-annotations)) > > So you propose to search for the face text-property in the provided string > to decide whether to add the default face in completion--insert-strings? Yes, the strings of the prefix/suffix. >> This gives the client full control over the visual appearance if that is >> preferred. Maybe this approach could also make sense to be included in >> Emacs? > > Do you see any possible backward-compatibility issues with changing this in > Emacs? For example, when a package like Selectrum puts another face > on the completion string, then it will be displayed instead of the default > completion-annotations face. We already do this for annotations/affixations in Selectrum but only based on the face of the annotation/affixation itself, the completion string doesn't affect this. I hope this wouldn't have any visual downsides for old code which assumes the faces get merged but I haven't encountered any cases where code tried to apply custom faces to annotations besides the marginalia package. Letting the client control it makes it easier to configure the display as it's hard to predict what will come out of face merging with the face the user has configured as `completion-annotations` face. This new behaviour could also only be applied for affixation functions to avoid any possibly bad effects of existing code. > Thanks for noticing the documentation problem. Do you think > this fix is sufficient: Looks good to me, too. Thank you!