From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Newsgroups: gmane.emacs.bugs Subject: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting Date: Fri, 13 Aug 2021 14:36:21 +0100 Message-ID: References: <3d3f894f-a6fa-53ae-5d50-c3aa9bffc73e@daniel-mendler.de> <56ab18b1-4348-9b2c-85bb-af9b76cd429a@daniel-mendler.de> <38a06f3c-4a7a-755c-c99b-708f91afabfa@daniel-mendler.de> <9f59f87c-2489-aaa0-5b3f-0e911b7ffa6c@daniel-mendler.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="27465"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 47711@debbugs.gnu.org, Stefan Monnier , 48841@debbugs.gnu.org, Dmitry Gutov To: Daniel Mendler Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Aug 13 15:37:12 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 1mEXN1-0006vW-BU for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 13 Aug 2021 15:37:11 +0200 Original-Received: from localhost ([::1]:55212 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mEXN0-00011m-5Y for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 13 Aug 2021 09:37:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:51580) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mEXMt-0000yw-7I for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 09:37:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57605) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mEXMs-0002Ux-VT for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 09:37:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mEXMs-0008D9-RM for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 09:37:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 13 Aug 2021 13:37: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.162886180231523 (code B ref 48841); Fri, 13 Aug 2021 13:37:02 +0000 Original-Received: (at 48841) by debbugs.gnu.org; 13 Aug 2021 13:36:42 +0000 Original-Received: from localhost ([127.0.0.1]:40917 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEXMY-0008CH-B5 for submit@debbugs.gnu.org; Fri, 13 Aug 2021 09:36:42 -0400 Original-Received: from mail-pl1-f179.google.com ([209.85.214.179]:43911) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEXMV-0008Bw-1X; Fri, 13 Aug 2021 09:36:40 -0400 Original-Received: by mail-pl1-f179.google.com with SMTP id e19so12007291pla.10; Fri, 13 Aug 2021 06:36:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=tvhjGLLU6CrPIl2BruGEyiA2A8Y1z7e+RBKklFm3qaY=; b=WO6Qd/orW+L6sc7R1/ProxKfGtNTRSoLWOpPCBg2HsS4hNQN0IfFCM9vtuwahg7YSc y5j9j8J5pFb7pZXkCmI5hJO73LhiVAQA9zwBpVbTCh5w41gt7X/C7slXlMm8cxfhzZVk t78oUsxDQXpqMBzVTKenBQTnAQC5LAvIUruT4OBagHR7s35sQXBrrAwUqkjHjz6UcyYa uBdIxbgTQ/wP9xejKyFASJfY4Xlvv45tPw55SXPaqn3qXjJ6Q5j4VBnpfDA8GH+Cr+zQ PmvqBJHw0y+T5zUd5debsLzFCMKOAkYZQ+zl8/YuyaW0TUIAetq0n859qgTHbpcNY27E uEAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=tvhjGLLU6CrPIl2BruGEyiA2A8Y1z7e+RBKklFm3qaY=; b=nr+vEnLGKC33Ilcv1/hcPHgqXiNnUKl+mvSF94J3uwqYpp/Ne2P+cJzvzNwfZpw92k cdfkGO2BM96+oxkMY+QffGxZDEKJXBfG2p/BodDe+Xvl1MrZJMqXQWma0LRdtZ7Ey0GC Va2bOGXHhocqRmn4pGVW8HDObXeOsF2ZXRDZk7ECgiyjMb30j4eGOSJpjTwXF5CHQQ65 HMzjz2e08rrA80Y7LAnOo9IjPlPzimx4FfJVbWaZi2Rfa1HunW5WdhJKVDqiJ8t5UjBP 9P22quEBpM+96aq0KYdjgIK8MvYA7ISrKT80Ujv5wOWJwUhgDcjHNu2MS1ucGLO5gBi1 1nsA== X-Gm-Message-State: AOAM531aKxIK6KX97S4/wGTRppidvp8nrLKDrhYgnZ+xSMX6Vzu5Ulaz ZFeANigcxIyqNHQaoxKIf+cxQsigik5eI8TlwB8= X-Google-Smtp-Source: ABdhPJyqcaYJLJRUlMaexDe0u7Fuq1zPF1MfteXDQLEs+nt9W0IdOoXkavODVEBpwkrwjUcylKHzniBr4TbNfnLRSis= X-Received: by 2002:aa7:9050:0:b029:3af:7e99:f48f with SMTP id n16-20020aa790500000b02903af7e99f48fmr2534645pfo.2.1628861793055; Fri, 13 Aug 2021 06:36:33 -0700 (PDT) In-Reply-To: <9f59f87c-2489-aaa0-5b3f-0e911b7ffa6c@daniel-mendler.de> 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:211763 Archived-At: didnOn Fri, Aug 13, 2021 at 1:56 PM Daniel Mendler wrote: > > On 8/13/21 2:37 PM, Jo=C3=A3o T=C3=A1vora wrote: > > On Fri, Aug 13, 2021 at 1:22 PM Daniel Mendler = wrote: > > > >> It is important to keep this property since this will preclude many bu= gs > >> due to string mutation. > > > > I am aware of this, of course. Can you give examples of these "many bu= gs"? > > Perhaps other than the one I already described and addressed? > > No, Jo=C3=A3o, this is not how it goes. I don't have to prove to you tha= t > your idea introduces bugs. So you just say it and I have to believe it? Then I could say the same to you, right? I won't of course, that would be silly. You have to show that mutation of the > completion table strings (which are not supposed to be mutated) will not > lead to bugs, which are hard to find. > > In contrast with the new API `completion-filter-completions` this entire > class of bugs is avoided by construction of the API. Furthermore the > `completion-filter-completions` API is easy to use in comparison to your > idea, where "compliant" backends have to apply string manipulations to > apply the highlighting and revert the strings back to their old pristine > state. The only thing the API user has to do is to call the `highlight` > function returned in the alist by `completion-filter-completions`. > > >> By separating the filtering and mutation > >> (highlighting, scoring) my patch addresses the problem at hand in the > >> proper way. > >> [ ... ] > >> Mutation would be a reasonable choice here if the problem could not be > >> solved in a proper way. But in fact it can be solved in a proper way > >> without mutating the strings at all as my patch shows. > > > > "proper" is just an reasonably empty adjective. There are different wa= ys to > > go about this, of course. What's "proper" and isn't is hard to debate > > objectively. > > You are contradicting yourself here. You agree that string mutation is > better be avoid. If we define "proper" as avoids string mutation if this > is easily possible, then my patch implements a proper solution to the> pr= oblem. I didn't say it's better avoided, though of course I will avoid _any_ chang= e if I can. I said I have identified one drawback with doing it. Then I have addressed that drawback. So that's what I said. I am unaware of _other_ drawbacks. They might exist, but I am unaware of them. Perhaps you are, and indeed you state they exist, but you refuse to let me know about them. Or perhaps others know of them and will let me kno= w. In my long-running discussion with Dmitry they were not presented (again, except for the one I identified). > > That's for sure. My patch idea addresses only that single problem. > > I think this is a good property of patches: to solve one thing, not man= y. > No, this is not necessarily true. This is only good if the problem is > solved in a way which is future proof. OK, but what thing of the future, real or academic, do you envision that would bring back the problem, or create other problems? > The idea of mutating the strings is a hack and not a solution. Without facts to back it up, I have to take this as gratuitous disparagemen= t. Nicht so gut. > In contrast, I am presenting a > future-proof new API as solution which addresses multiple problems. That's the issue. The completion system is very complex and there are many good ideas, different, floated by many people. But if you make a patch to address "multiple" fuzzily-described problems, it's hard to judge how good your ideas even are! Maybe they are indeed very good, I never said they weren't. No need to get worked up about it! Again, my proposal is to first focus on the performance problems caused by string allocation. _That_ problem is well understood, at least by me (but = it would help to settle on convenient benchmarks understood by others, too). Then we can go from there. > you look at the patch, only 196 new lines are added to minibuffer.el. > Furthermore the patch adds 213 lines of new tests. It's a large patch, over 1000 lines. One does not review a patch merely by looking at lines added, when one needs to read much more, to understand implications, = etc. It needs documentation, for one, much more than just docstrings, on how to use the new API. > Jo=C3=A3o, you don't have to lecture me on these things. Of course I can > provide such numbers. Then please do! Not meaning to lecture you, just that your suggestion that I try Vertico UI as a substitution for these numbers seemed completely misguided. So if you have them (or "can provide them") let's see them. All I'm asking, preferably from Emacs -Q recipe. > You cannot reasonably make the claim that > `copy-sequence` is the problem and at the same time claim that my patch > does not solve the performance issues, when in fact my patch avoids this > exact string copying. I didn't say it didn't solve them! Now, where did I say that? I would like to see a benchmark so that I can witness it _and_ study alternative solutions. With that, there's a better chance that I will be persuaded there are none as elegant, clean, proper, pure, etc as yours! Maybe others review patches on other aspects that's fine. Maybe others will. Eli reviewed on minor formatting and documentation aspects. I review them on substance, using numbers and conducting my own experiments and tests. This takes time and help from the scientist on the other end. Simple and in summary, let's hope your next reply has some benchmarks so we can make progress. Thanks, Jo=C3=A3o