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.bugs Subject: bug#47711: bug#48841: [PATCH] Add new `completion-filter-completions` API and deferred highlighting Date: Thu, 12 Aug 2021 10:47:17 +0200 Message-ID: References: <837dgrdrec.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="33992"; mail-complaints-to="usenet@ciao.gmane.io" Cc: joaotavora@gmail.com, dgutov@yandex.ru, monnier@iro.umontreal.ca, 48841@debbugs.gnu.org, 47711@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Aug 12 10:48:10 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 1mE6Nm-0008aw-9O for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 12 Aug 2021 10:48:10 +0200 Original-Received: from localhost ([::1]:53056 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mE6Nk-0000w3-Sa for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 12 Aug 2021 04:48:08 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36852) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mE6Ne-0000vk-Bg for bug-gnu-emacs@gnu.org; Thu, 12 Aug 2021 04:48:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:54211) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mE6Ne-0001Qi-1b for bug-gnu-emacs@gnu.org; Thu, 12 Aug 2021 04:48:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mE6Nd-0000Pi-Tx for bug-gnu-emacs@gnu.org; Thu, 12 Aug 2021 04:48:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Daniel Mendler Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 12 Aug 2021 08:48:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47711 X-GNU-PR-Package: emacs Original-Received: via spool by 47711-submit@debbugs.gnu.org id=B47711.16287580481536 (code B ref 47711); Thu, 12 Aug 2021 08:48:01 +0000 Original-Received: (at 47711) by debbugs.gnu.org; 12 Aug 2021 08:47:28 +0000 Original-Received: from localhost ([127.0.0.1]:37522 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mE6N6-0000Oh-CB for submit@debbugs.gnu.org; Thu, 12 Aug 2021 04:47:28 -0400 Original-Received: from server.qxqx.de ([178.63.65.180]:51005 helo=mail.qxqx.de) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mE6N5-0000OR-Aa; Thu, 12 Aug 2021 04:47:27 -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=i2BeI1W5p4ITvwSsYZbpZMP46bquNh7WtZnDbLxnDTs=; b=CkXnzKP04T4dpOSSH2ZJ+E38aD YyMJjNahZq7mYM0sODxvUmJLg0HxHEwRzrUnlWF+2rHz9e1q7O4WlG8pPrsbBCyJ/2rOY5IXaMZy5 AtgefXe2+civGL9/WP4+huRNqrnjQtnWrWEvRL+u+OnHMqHcrVrd8SGqA5EUYts6Zv/8=; In-Reply-To: <837dgrdrec.fsf@gnu.org> 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:211654 Archived-At: Eli, thank you for your feedback and for pointing me to the mode which helps with the formatting. I will address the documentation and formatting issues as soon as the discussion has concluded. In the following I answer to a few of your questions about technical details. >> The `completions` value is the list of completion strings *without* >> applied highlighting. The completion strings are returned unmodified, >> which avoids allocations and results in performance gains for > > This is unclear: how can you return a list of strings which you > produce without allocating the strings? The function 'completion-filter-completions' receives a completion table as argument. The strings produced by this table are returned unmodified, but of course the completion table has to produce them. For a static completion table (e.g., in the simplest case a list of strings) the completion table itself will not allocate strings. In this scenario 'completion-filter-completions' will not perform any string allocations, only the list will be allocated. This is what leads to major performance gains. >> +(defvar completion--filter-completions nil >> + "Enable the new completions return value format. > > Btw, why is this an internal variable? Shouldn't all completion > styles ideally support it? If so, it should be a public variable, > documented in the ELisp manual. And the name should also end with -p, > since it's a boolean. How about completion-filter-completions-format-p? (As I understood the style guide '-p' is not a good idea for boolean variables, since a value is not a predicate in a strict sense.) To address your technical comment - this variable is precisely what one of the technical difficulties mentioned in my other mail is about. The question is how we can retain backward compatibility of the completion style 'all' functions, e.g., 'completion-basic-all-completions', while still allowing the function to return the newly introduced alist format with more data, which enables 'completion-filter-completions' to perform the efficient deferred highlighting. > Also, the "This function has been superseded..." part should be a new > paragraph, so that it stands out. (And I'm not yet sure we indeed > want to say "superseded" here, but that's part of the on-going > discussion. maybe use a more neutral language here, like "See also".) The new API 'completion-filter-completions' will substitute the existing API 'completion-all-completions'. I only didn't go as far as deprecating the 'completion-all-completions' API right away, but we could also do this. > Is "filter" really the right word here (in the doc string)? "Filer" > means you take a sequence and produce another sequence with some > members removed. That's not what this API does, is it? Suggest to > use a different name, like completion-completions-alist or > completion-all-completions-as-alist. "Filter" seems like exactly the right word to me. The function takes a list of strings (or a completion table) and returns a subset of matching completion strings without further modifications to the strings. See above what I wrote about allocations. >> +Only the elements of table that satisfy predicate PRED are considered. >> +POINT is the position of point within STRING. The METADATA may be >> +modified by the completion style. The return value is a alist with >> +the keys: >> + >> +- base: Base position of the completion (from the start of STRING) > > "Base" here means the beginning? If so, why not call it "beg" or > somesuch? Base position is a fixed term which is already used in minibuffer.el for completions. See also 'completion-base-position' for example. > Are we really losing the completion-score property here? If so, why? Yes, the property is removed in the current patch. It is not actually used for anything in the new implementation. But it is possible to restore the property such that 'completion-all-completions' always returns scored candidates as it does now. See my other mail regarding the caveats of the current patch. Daniel