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#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting Date: Mon, 16 Aug 2021 14:05:54 +0200 Message-ID: References: <9f59f87c-2489-aaa0-5b3f-0e911b7ffa6c@daniel-mendler.de> <8a36e61a-1c5b-bf3b-a454-077348589c4f@yandex.ru> <87y29471ov.fsf@gmail.com> <837dgob6yo.fsf@gnu.org> <87wnootec0.fsf@gnus.org> <5d70b0ad-3838-ddb8-d341-9a5531d9da73@yandex.ru> <0cbf224b-b382-8a02-3f06-9f6d7e5e9741@yandex.ru> <87a6lihv7b.fsf@gmail.com> <63e03b18-db81-3b11-c692-0af9df20c506@daniel-mendler.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="29834"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Stefan Monnier , 48841@debbugs.gnu.org, Dmitry Gutov , Lars Ingebrigtsen , 47711@debbugs.gnu.org To: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Aug 16 14:07:14 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 1mFbOb-0007Sa-DN for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 16 Aug 2021 14:07:13 +0200 Original-Received: from localhost ([::1]:41766 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mFbOa-0008U6-B0 for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 16 Aug 2021 08:07:12 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:57532) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mFbOQ-0008Q0-N2 for bug-gnu-emacs@gnu.org; Mon, 16 Aug 2021 08:07:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:37177) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mFbOQ-0004WC-Gm for bug-gnu-emacs@gnu.org; Mon, 16 Aug 2021 08:07:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mFbOQ-0002o8-CA for bug-gnu-emacs@gnu.org; Mon, 16 Aug 2021 08:07:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Daniel Mendler Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 16 Aug 2021 12:07:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 48841 X-GNU-PR-Package: emacs Original-Received: via spool by 48841-submit@debbugs.gnu.org id=B48841.162911556610695 (code B ref 48841); Mon, 16 Aug 2021 12:07:02 +0000 Original-Received: (at 48841) by debbugs.gnu.org; 16 Aug 2021 12:06:06 +0000 Original-Received: from localhost ([127.0.0.1]:48720 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mFbNV-0002mO-GU for submit@debbugs.gnu.org; Mon, 16 Aug 2021 08:06:05 -0400 Original-Received: from server.qxqx.de ([178.63.65.180]:59331 helo=mail.qxqx.de) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mFbNT-0002lo-8G; Mon, 16 Aug 2021 08:06:04 -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=Rqm3CwKvaY3irAWRkNIM3r7NyHLM9cGe9M3IfOA8XxM=; b=ktVOSLa8U2WRAXCOadinumlvJp FRn5yd3JtduX91ZpaoVdXSgrjRtx+cemkLkO/k7UphFRNq9cBPahXSsImb8uapcbODMEXeIhBRnGH oh+2hJ/SFUU7usXD9ubtxJrfKs4zQGlYeBC5I+w3mut2ZG26VRhpJIwkn95aORXMAX2s=; In-Reply-To: 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:211991 Archived-At: João, the discussion is clearly not progressing. I propose that we both take a step back and let the Emacs maintainers, who participated in this discussion, decide on how to proceed. It seems to me that all arguments and data has been presented and there is no need for further reiterations in more and more colorful language. I would also like to point out that implying that I don't understand your language is borderline acceptable. I understand the discussion very well, but I don't understand why you are using these unfair and invalid means of discussion. For example there could be these decision outcomes: 1. The information presented up to now does not allow the maintainers to make a decision. For example the maintainers may ask for further clarification from you, João, or they may ask for benchmarks from me or a prove that my patch does not lead to performance regressions. 2. The maintainers decide that no patch should be merged. 2. The maintainers decide that your patch will be merged. I will accept this decision. 3. The maintainers decide that my patch will be merged. You will accept this decision. 4. The maintainers decide that both patches will be merged such that both approaches will be supported. We both will accept this decision. I want to summarize the situation in the following: The patches in question address a performance issue in the current completion machinery which is caused by over-eager copying of the completion candidate strings and over-eager application of the highlighting to all candidate strings. For incrementally updating UIs it would be sufficient to only copy and highlight the strings which are actually going to be displayed. My patch takes the approach to expose the existing two-step completion process, which consists of filtering and highlighting. By returning the filtered completion strings and a highlighting function this two-step process is decomposed and the caller of the API has the ability to call the highlighting function on only the displayed subset of completion candidates. I argue that exposing the filtering and highlighting procession steps is the logical and natural conclusion of the existing machinery. My patch is fully backward compatible and aims to not introduce any regressions (also no performance regressions) to the existing API. Furthermore my patch adheres to the current guarantees given by the existing 'completion-all-completions' API. The completion strings provided by the completion backend are not mutated in any way, no string properties are attached. Since the API 'completion-filter-completions' proposed in my patch does the minimal amount of work necessary (only filtering), if no highlighting is requested, I argue that the new API is the most efficient possible API, given the current constraints. Furthermore since I am introducing a new API, outstanding issues can be solved which could not be solved until now given the constraints of the existing 'completion-all-completions' API. In particular the new API 'completion-filter-completions' API returns additional data like the end position of the completion boundaries. Until now the end position was not made available and 'completion-base-position' just used the length of the input to guess the end position. In a strict sense this guess is incorrect and there is a FIXME in minibuffer.el, mentioning this issue. The downside of my patch is that it is a large patch. While it adds only 196 lines of code, which is not much and expected given that it only reshuffles the existing machinery, it is still a large patch in total. On the other side, João's patch avoids the complication of adhering to the existing guarantees of the APIs and takes the liberty of attaching "private" string properties to the completion strings of the completion table backend. I argue that attaching the string properties is a violation of the guarantees of the existing API and violates the expectations of the existing many completion tables. One very severe scenario is when obarray is used as completion table, since then each symbol name gets a private property attached. I argue that such global side effects like attaching string properties to all completion candidates should better be avoided. There is the issue that the attached string properties are a potential memory leak. When dumping the string representation of symbol names, the symbols will have additional properties which will complicate the debugging experience. Furthermore it will lead to confusion since the global side effect during completion will suddenly have an influence of symbols which don't have to do anything with completion. The big advantage of João's patch is that it is very limited in scope and very simple. However I argue that this simplicity is hard-won and we will regret this approach later due to the global side effects. Therefore I conclude that the two-step process proposed in my patch, which does not introduce problematic global side effects is the better approach forward. Furthermore a new API is needed such that more completion data can be returned, e.g., the completion end position. One could even return additional match data in the future given that the new API 'completion-filter-completions' is extensible thanks to its alist return value. João, please feel free to also present your closing arguments here. Daniel