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 13:37:38 +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> 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="19886"; 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 14:38:34 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 1mEWSI-0004zA-AY for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 13 Aug 2021 14:38:34 +0200 Original-Received: from localhost ([::1]:54568 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mEWSH-0000pr-8C for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 13 Aug 2021 08:38:33 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:39226) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mEWRn-0000pL-3c for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 08:38:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57468) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mEWRm-0003lX-T3 for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 08:38:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mEWRm-000062-QT for bug-gnu-emacs@gnu.org; Fri, 13 Aug 2021 08:38: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 12:38: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.1628858279345 (code B ref 48841); Fri, 13 Aug 2021 12:38:02 +0000 Original-Received: (at 48841) by debbugs.gnu.org; 13 Aug 2021 12:37:59 +0000 Original-Received: from localhost ([127.0.0.1]:40779 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEWRi-00005O-Lf for submit@debbugs.gnu.org; Fri, 13 Aug 2021 08:37:59 -0400 Original-Received: from mail-pj1-f54.google.com ([209.85.216.54]:50968) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEWRh-00004t-9k; Fri, 13 Aug 2021 08:37:57 -0400 Original-Received: by mail-pj1-f54.google.com with SMTP id bo18so15156570pjb.0; Fri, 13 Aug 2021 05:37:57 -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=tlFNsJoHhRBkUHPWY4LrARaxXuMGniqUKbtu2G0qdXU=; b=W2jhGqYmQiddIO9xrhNq+mgLhNpZDrXgB7E3Nq9ikxl01hbFX3Xd7DKIjge5nk70Vk TJ6lEut6rRtmflXg2bZWmdKRLTVld535iIGhkPNPkR2iE5mzIBImO16T3hpJMGEAqQv7 Yhr8k8CWpX1DATiwscnuWOKn8mfY2+OzkPrdykjc/3wfXZb8o3M/yKk0NdfzqtI6ymFg 0kPcVTzA66bPapJPUHB8ZIrhZztHJy3tmpAGVeMOUngbv1wGsxMvUTRwlZe1YsLBzK6+ KKaqcpLVT2U+JgodurScZY/iVu9Yzg7kQaUXfr3xSsiXh8bXW9UwtkjVdPzFp2sSSxI8 eG/w== 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=tlFNsJoHhRBkUHPWY4LrARaxXuMGniqUKbtu2G0qdXU=; b=MSUMwNM1xDgUgyDOyycbk6t2dYHgo4sV0XHsatXgNpSvKZ9YH34CNMmfKZKZ19xLfW M7OfsOt3tHBGN3g95s2YqHPo6swiNlHncTqBJrwIhlJcrTT2Yqx8BiHduUuMGrutw5+h VUeHt52P8EcFYZFdgNbtZbLToqq+Mru0MghAtWGYwpI5CyRZYrKB2qesDKpv+vcfIwUs HqVtM54gdxBnGzPgKeGuu/W9kkqKyhdTwRmhsDGfwz8NTTfconxgRhFcjaOOvO6agmaz Y6MlnngG4cM0S3vjLFWM/gIDIcDQBZbxVY3BzPILmCN2m9RRa5DOVhWCY1DpRJ5s9kZN NgPA== X-Gm-Message-State: AOAM533LyFHgkB8NdTtQk9u31KuoZZ2wotSW8j8BVW/NqfHWrsnUup2m aSTQuKFoIbnP+NZEcJXQVbU1f7js7LJI7X9N18w= X-Google-Smtp-Source: ABdhPJydEtmy84BK6Wfd+Bbslze/kSoY9HX0jvqeGGlmCcf/udvaNVKD9YyLf4HvKp8/XJpyzqs4997sPyThwdhiEiE= X-Received: by 2002:a62:5c6:0:b029:341:e0b1:a72c with SMTP id 189-20020a6205c60000b0290341e0b1a72cmr2220586pff.71.1628858271491; Fri, 13 Aug 2021 05:37:51 -0700 (PDT) In-Reply-To: <38a06f3c-4a7a-755c-c99b-708f91afabfa@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:211750 Archived-At: On Fri, Aug 13, 2021 at 1:22 PM Daniel Mendler wro= te: > It is important to keep this property since this will preclude many bugs > due to string mutation. I am aware of this, of course. Can you give examples of these "many bugs"? Perhaps other than the one I already described and addressed? > 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 ways t= o go about this, of course. What's "proper" and isn't is hard to debate objectively. > This solution is much more ad-hoc and you still mutate the string which > is not allowed. It's also difficult to debate "ad-hoc" or not. If you've studied the problem, what makes you say that mutating the string (in this case, adding a 'completion--style-face' property to it) is not allowed? What negative thin= gs would derive from it. > > The advantage that I see is that those adaptations apart, it is a small > > localized and effective change. > > Note that your idea also does not address the other issues which are > addressed by my patch. 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 many. We can make more patches to solve other problems, once we identify them clearly. > The new API `completion-filter-completions` > returns data which hasn't been available before, e.g., the end position, > which cannot be fixed given the existing API. > > >> The main problem is that `completion-all-completions` allocates all th= e > >> strings every time the completions are filtered. This is the same > >> performance issue you encountered in fido-mode/icomplete-mode. > > > > OK. I encountered at least two different performance problems there, wi= th > > quite different causes. So let's stick to the string-allocation problem= . Post > > a code snippet that demonstrates the problem the way you see it/experie= nce it? Look, one needs to evaluate things quantitively. Your patch is not to Vertico, it's to Emacs. I'm concerned with changes to Emacs and their effect on all completion frontends. So trying Vertico isn't very useful. If you're solving a performance problem (and it seems that you are, among other things) we really need benchmarks, a description of an experiment who= se results can be reproduced independently. It's the normal scientific method. Something like: "before my patch, this code takes 123 seconds to run, after my patch it takes 12." > >> The second problem addressed by the new API > >> `completion-filter-completions` is that `completion-all-completions` i= s > >> limited in what it can return. For example it cannot return the end > >> position of the completion. > > > > And why is this a problem? Can you post an example of something you'd > > like to do, but can't? Regardless, it does seem indeed like a "second"= problem > > (as you state) so perhaps something that can be addressed separately. > > Please look at the FIXMEs in minibuffer.el which address this. > Currently only the beginning position of the completion boundary is > returned, which is only half of the information. OK. It does seem like a separate problem, so maybe open a new bug for it? Jo=C3=A3o T=C3=A1vora