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] (icomplete-vertical-mode): Add support for affixations and, annotations Date: Tue, 1 Jun 2021 21:03:12 +0200 Message-ID: <4649e8f9-7ac2-1d5e-5a44-90e470d6d2ad@daniel-mendler.de> References: <87h7irss50.fsf@mail.linkov.net> <43d1599e-2ba9-2efb-45c3-76c67d29a69d@daniel-mendler.de> <87tumrgqrb.fsf@gmail.com> <87tumq92pe.fsf@mail.linkov.net> <87lf82g10g.fsf@gmail.com> <87y2c24lww.fsf@mail.linkov.net> <871r9t2lsy.fsf@mail.linkov.net> <22880197-6d05-c821-2c58-328ed3cfc02e@daniel-mendler.de> <87eedruui3.fsf@gmail.com> <8dd915fe-fe67-2a45-67ff-8aaa3e9b9aca@daniel-mendler.de> <878s3zuq47.fsf@gmail.com> <09f2a253-84ba-5cfd-552e-0b89109864a5@daniel-mendler.de> <875yyxaoxp.fsf@gmail.com> <871r9laj6a.fsf@gmail.com> <1b73a130-204c-76fb-2b60-02b814aee0f0@daniel-mendler.de> <87r1hl8xom.fsf@gmail.com> <878s3t8tzw.fsf@gmail.com> <3c68bd00-70ca-fa18-f9b8-cd03029f9294@daniel-mendler.de> <8735u18lsd.fsf@gmail.com> 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="22256"; mail-complaints-to="usenet@ciao.gmane.io" Cc: "emacs-devel@gnu.org" , Dmitry Gutov , monnier@iro.umontreal.ca, Juri Linkov To: =?UTF-8?B?Sm/Do28gVMOhdm9yYQ==?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Jun 01 21:04:35 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 1lo9gp-0005YZ-Cg for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Jun 2021 21:04:35 +0200 Original-Received: from localhost ([::1]:52414 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lo9go-0004na-8S for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Jun 2021 15:04:34 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:55932) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lo9fi-00046h-9h for emacs-devel@gnu.org; Tue, 01 Jun 2021 15:03:26 -0400 Original-Received: from server.qxqx.de ([2a01:4f8:121:346::180]:60193 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 1lo9fb-0006j2-Hm for emacs-devel@gnu.org; Tue, 01 Jun 2021 15:03:25 -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=nJixXMleTIR7bO3Plv5O0igtTQfEc8rcOwscJN5AKTM=; b=vhtDMHMIeem/3eLxlunZJhDh2t KRz8Wp5keZeU1kQkumRkHwgGpZCukoYMJq4k7GutrCsEryixhTNJjMKsh1M1+zHDIN9mZ17sTRapa KrADHCT9C4K/FPqoDbeiGcPb7GVJMXlv8UotjGQ5p7zZB+RV2ewhra68CSoMM78gvr1w=; In-Reply-To: <8735u18lsd.fsf@gmail.com> 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:270246 Archived-At: On 6/1/21 8:47 PM, João Távora wrote: > I really doubt this "matters when scrolling", as grouping would be > called once per filtering, and scrolling is just getting at each > candidate's transform. If anything, if you take the last option, it > could _speed up_ scrolling (and the cost of some more initial latency). Yes, you are right about that. It matters when refiltering the candidate list, which happens quite often in Vertico or other continuously updating UIs. >>>> The current implementation is simple enough and also avoids allocation >>>> of the cons pair and the allocation of the lambda as in your proposal. >>>> The efficiency is crucial here. >>> >>> I believe you, but I haven't seen any evidence (yet) to back up the >>> claim. But instead of me whistling premature optimization song, feel >>> free to point me to some numbers or something. >> >> The current version is proven in my packages and performs well. I don't >> see a point in replacing it to a more complicated and less efficient >> version. > > I've told you the point. A simpler minibuffer.el code, possibly with > less branching and logic, which might even be faster. I doubt that you manage to make it simpler. If you indeed manage to make you simpler I will propose a counter patch based on the current definition which is as simple. Generally, we should not base our design on the internals of the UI but rather on the ease of use and the performance of the API. The internals can certainly be made nicer, but this is an orthogonal discussions. I welcome simplifications and improvements. I also submitted some patches in this direction myself. In the process of adding the `group-function` I made such simplifications. >>>> Furthermore I argue that the current implementation is simpler to >>>> understand for the completion table authors than your counter >>>> proposal, which introduces a deferred computation with the lambda. >>> >>> It's like the affixation-function thing, I don't think it makes sense >>> for client code to do these IFs, much as I dont' think it makes sense >>> for them to do MAPCARs. >> >> No it is not. > > Look, this is just my opinion, you don't need to concur with it it, but > there's no need to say "no it's not" to an opinion. There is also no need to rediscuss a fine solution without making a better and proper counter proposal. I told you that I care about this being efficient - meaning no allocations. If your proposal fails to address this, I will oppose it. >> completion table authors. Furthermore I disagree with you that the >> minibuffer.el code is messy. > > Surely, you jest, lol. minibuffer-completion-help is a mess. That's > ok! Most of minibuffer.el is a mess. There's no good code, in general, > so let's not get emotionally attached to ours. I am talking about the added complexity due to the `group-function` of course. I am not emotionally attached to the minibuffer code and neither to my code. I suggest you keep the discussion on a technical level. >> The suggestion of yours to mutate the candidates by putting a property >> instead of threading the group function through the call sites is much >> worse. > > In what way? Really, all other things being equal, in what way is > threading arguments through lots of functions, bloating the arglist, > implementations and the docstrings of said functions, calling group-fun > multiple times and in multiple places in minibuffer.el better than _not_ > doing those things? Please present your counter proposal which is as efficient and which is simpler. Then we can discuss. But I will then make another proposal which will simplify minibuffer.el as much as your proposal. > The only argument is if there was a terrible performance cost, which was > indeed the first and only real argument you advanced. But I've not seen > evidence of that reality. I benchmarked this heavily in my Vertico UI. So I can assure you that I looked into this. I rely on this for the performance of Vertico and for the performance of other continuously updating UIs. The current group-function is implemented in Selectrum (MELPA), Vertico (GNU ELPA), Icomplete-vertical (GNU MELPA) and used by Emacs and my Consult package so far. It is a proven implementation. I don't think it is justified to reiterate the whole discussion. At some point it must be possible to rely on the code which made it into Emacs master. >> Your version is not an improvement. Please read through the old >> discussion and consider my arguments. I also don't see a point in >> rediscussing this as long as you don't make a proposal which is a clear >> improvement. > > Look, I get it that you oppose this suggestion and are happy with your > implementation. That's fine it's your right to be. But there's no > point in so aggresively gainsaying a suggestion while not offering any > material arguments. APIs in master aren't meant to cristalize, they are > meant to be iterated. I have made many material arguments and you ignore them. The status quo is - we have the 'group-function` in the current form. While not being emotionally attached to it, I think it is a fine implementation. If you want to change it, make a better proposal. I will oppose it as long as it does not address my performance concerns. If it addresses these concerns I am of course open to improvements. I am the last person to say no to a technical argument. You are the one who did not bring a good one here, except some criticism regarding the current way minibuffer.el is implemented. This can be improved and I think we can do that in a way keeping the current definition of the `group-function`. Daniel