From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Daniel Mendler Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] `affixation-function`: Allow only three-element lists Date: Sun, 25 Apr 2021 20:02:39 +0200 Message-ID: <02eb444c-d8e2-8271-1edc-b02b3ce7f64c@daniel-mendler.de> References: <87h7jujmbn.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="22954"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Stefan Monnier , emacs-devel@gnu.org To: Juri Linkov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Apr 25 20:03:50 2021 Return-path: Envelope-to: ged-emacs-devel@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 1laj6k-0005sy-Qc for ged-emacs-devel@m.gmane-mx.org; Sun, 25 Apr 2021 20:03:50 +0200 Original-Received: from localhost ([::1]:53704 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1laj6j-0003IT-U0 for ged-emacs-devel@m.gmane-mx.org; Sun, 25 Apr 2021 14:03:49 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46210) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1laj5k-0002rc-BY for emacs-devel@gnu.org; Sun, 25 Apr 2021 14:02:48 -0400 Original-Received: from server.qxqx.de ([2a01:4f8:121:346::180]:54719 helo=mail.qxqx.de) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1laj5g-0008EC-1f for emacs-devel@gnu.org; Sun, 25 Apr 2021 14:02:47 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=qxqx.de; s=mail1392553390; h=Content-Transfer-Encoding:Content-Type:In-Reply-To: MIME-Version:Date:Message-ID:From:References:Cc:To:Subject:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=rORER6UPm2k2r4r3dy7RjH2fGccrCoAyN2GEZS0+0DA=; b=gYOi32ZByW01RaWJ9Q8/KywnKv 3uyVPUTg+ihQgKBYpxJ+ckg1PB5gp8aMc4sUEBnVc5ibptO9kwa9ALRXzEyAw+cBAYvkdGOtliFp2 4RT/wBTe2ngsv5RCXMQiM+mzB7GvvvxYuBmWs7qfx+nWJCFYmECBPOt2sL4iUoN1yQJc=; In-Reply-To: <87h7jujmbn.fsf@mail.linkov.net> Content-Language: en-US Received-SPF: pass client-ip=2a01:4f8:121:346::180; envelope-from=mail@daniel-mendler.de; helo=mail.qxqx.de X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action 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-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:268405 Archived-At: On 4/25/21 7:46 PM, Juri Linkov wrote: >> @@ -2110,7 +2110,9 @@ minibuffer-completion-help >> (cond >> (aff-fun >> (setq completions >> - (funcall aff-fun completions))) >> + (funcall aff-fun completions)) >> + (cl-assert (or (not completions) >> + (= 3 (length (car completions)))))) > > It's surprising to see that you added such artificial restriction. > I expected a change with something more like this: > > and then simplify the implementation of completion--insert-strings > by removing all complexity that handled suffix-only completion strings. Yes, this is certainly a less artificial way to restrict the results of the `affixation-function`. However this works only if you add the condition on when to add annotation faces as you proposed below. >> diff --git a/lisp/simple.el b/lisp/simple.el >> index 999755a642..26eb8cad7f 100644 >> --- a/lisp/simple.el >> +++ b/lisp/simple.el >> @@ -2080,8 +2080,11 @@ read-extended-command--affixation >> (obsolete >> (format " (%s)" (car obsolete))) >> ((and binding (not (stringp binding))) >> - (format " (%s)" (key-description binding)))))) >> - (if suffix (list command-name suffix) command-name))) >> + (format " (%s)" (key-description binding))) >> + (t "")))) >> + (put-text-property 0 (length suffix) >> + 'face 'completions-annotations suffix) >> + (list command-name "" suffix))) >> command-names))) > > Why such change only for one particular use of annotations? > Shouldn't completion--insert-strings check if the suffix is an > empty string, and then put the 'completions-annotations' face on it? I did not do this since I wanted to avoid magic which complicates the definition of the `affixation-function`. The idea was to never add faces to the annotations supplied by the `affixation-function`. I am looking at the `affixation-function` from the perspective of how the definition can be made as simple/predicatable as possible for the API user since there are other completion UIs and many completion tables which will make use of this. I treat the completion UI behind the API as a blackbox. In contrast you take the perspective of how the affixation function could be fitted best into the existing completion code without introducing artificial restrictions. I argue that it is better to take a step back from the actual code base in minibuffer.el and adopt the simpler definition. Daniel