From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Gregory Heytings Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and, annotations Date: Tue, 01 Jun 2021 14:47:43 +0000 Message-ID: References: <87zgwlb4xc.fsf@gmail.com> <617d06ca-27bf-2ae8-26eb-1042123499d3@daniel-mendler.de> <87pmxhb1rs.fsf@gmail.com> <23510125-37b9-e87e-3590-5322f44772ce@daniel-mendler.de> <87y2c5nhsr.fsf@mail.linkov.net> <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> Mime-Version: 1.0 Content-Type: text/plain; format=flowed; charset=us-ascii Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="34990"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: =?UTF-8?Q?Jo=C3=A3o_T=C3=A1vora?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Jun 01 16:48:30 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 1lo5gy-0008ql-M4 for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Jun 2021 16:48:28 +0200 Original-Received: from localhost ([::1]:47804 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lo5gx-0006nb-Nj for ged-emacs-devel@m.gmane-mx.org; Tue, 01 Jun 2021 10:48:27 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:32820) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lo5gL-00065G-IL for emacs-devel@gnu.org; Tue, 01 Jun 2021 10:47:49 -0400 Original-Received: from heytings.org ([95.142.160.155]:49396) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lo5gJ-00022u-4Y for emacs-devel@gnu.org; Tue, 01 Jun 2021 10:47:49 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=heytings.org; s=20210101; t=1622558863; bh=uksagugfXmQ7Jtn65aZubyEMoy622FB0wXqy1ru92ZQ=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References:From; b=IakUSo+R4UEweSQsbxQAWZO80F/POxnJWmjQGXK8w1yK82hSaH6YNn6BHEKfoztwb tqNfc1nnT4HFZVM919vMPyAYHaJYuWfXwcCJK3IukMzZOQNTV1+JF/n9rJbcEI3yc+ q0qDMOFoLRDTLgXA31HsBnT4hMcPWYN1Wiw9+GX5YMGCEuT8xXNb44dUilFU4T8WKR l0qSSsjLSzve/F3f5Xl9TP51cCSF1vm81U88xAbhEiSSP43D/orBgfBH1Q8eOFir/Z 7AiKt/AzGIMRV+2+IfVEeNFHGrmLy5UCQeO0zQczswgJt0MWsF2FqidpqWC4efCeqC fpOep0L1CLnEg== In-Reply-To: <875yyxaoxp.fsf@gmail.com> Received-SPF: pass client-ip=95.142.160.155; envelope-from=gregory@heytings.org; helo=heytings.org X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, 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:270199 Archived-At: >>> I didn't get a terrible amount of feedback, but I spent the week >>> testing that branch, making small adjustments, and verifying that the >>> default Icomplete and Fido behaviour are unchanged. So I've now >>> pushed the icomplete-vertical-mode rework to master. >> >> Okay, so I guess it's too late to send you feedback... I asked you to >> wait a few days more, and promised to send you feedback this week. > > You can still send feedback, of course. Just because it's in master > doesn't mean it's written in stone. > That's most often the case, but let's see whether this one is an exception. > > I did wait a few days more to your request but I didn't think it made > sense to wait many more as I need to move on to other developments that > kind of depend on it. Anyway, I think you'll like the new mode, but let > me know what you think. > I've now spent some time to test the feature and to look at your code, and here are a few comments: 1. Your code is not modular enough, for example, IMO it would be much better if the the annotation function and the "candidate number/total number of candidates" indication were (user-configurable) hooks, instead of hard-coding these operations in the code. Not everyone wants to see these numbers, not everyone wants to see annotations even if they are available, some might want to add annotations to the candidates list with their own chosen formatting, and so forth. 2. I don't understand why you used "icomplete-scroll" for the variable name, and why it is not a defcustom. IMO it should have been something more explicit like "icomplete-vertical-scroll-candidate-list". And with a hook, this variable would not be necessary. 3. As I feared, the code to scroll the candidates list does not work correctly alas. I've put lots of efforts to make "icomplete-vertical" work correctly in virtually every possible case, so I find this very regrettable. For example, with frame-height = 47, 10 candidates are displayed, yet the candidates list rotates on the 7th candidate (instead of the 11th one). With frame-height = 30, 6 candidates are displayed, and the candidates list rotates on the 6th candidate (instead of the 7th one). With frame-height = 22, 4 candidates are displayed, and the candidates list correctly rotates on the 5th candidate. With frame-height = 15, two candidates are displayed, yet the candidates list rotates on the 4th candidate (instead of the 3th one, and in this case you don't even see the current candidate anymore). If I start working on this (again), I would likely change most of your code into something else, and it would take some time, because it's a difficult problem. Would you agree with this?