From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Ergus Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Completion preview common Date: Wed, 10 Apr 2024 17:54:15 +0200 Message-ID: References: <75CFBF4C-EA88-4985-A967-BCC896A60EDE@aol.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="1871"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Eshel Yaron Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Apr 10 17:55:23 2024 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 1ruaIE-0000CG-Ny for ged-emacs-devel@m.gmane-mx.org; Wed, 10 Apr 2024 17:55:22 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ruaHO-0004Go-Ix; Wed, 10 Apr 2024 11:54:30 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ruaHM-0004GQ-Jb for emacs-devel@gnu.org; Wed, 10 Apr 2024 11:54:28 -0400 Original-Received: from sonic308-2.consmr.mail.bf2.yahoo.com ([74.6.130.41]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ruaHI-0000IV-FU for emacs-devel@gnu.org; Wed, 10 Apr 2024 11:54:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aol.com; s=a2048; t=1712764460; bh=oLt6KKBmoEx669ELm/RIPiNJREudz9lKHlIShwo9SNE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From:Subject:Reply-To; b=BlPcnehtIZ6TglrCQleW/4F271Bu4LLQWeirTxMgu3Tx7vVKIInWDMMZ+b8U8bbuk+GyHxws7jIGoUTY+GZ3r0TZylk0k4f8QR/ML7rX7YKlwm/V09i5WwQL+JbBpCN3i1FXPOtt7HXnbc9Ybm2B/C8XgVzk+D2i7BcYums1J2Mhqx9DHgoOYiG/vAyUh3pV7vhIl8E9jtJ7+J9mXWqR9suAJfz7Q4h7xRZD+4XKzT/hjsNEcdNWb3QfCJHqMUXeimRvmzHzs2bcVVLBmCLhYS2JdxkyQBZquw8sXf7kYzA8kQX786ZqKdbvn8LTZFIBuvXbUsmcPrBoTnXupuXbpg== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1712764460; bh=IDFq7U+64+yPYcfo2G1PhvWHdVor1enKuDMFdt6ecAh=; h=X-Sonic-MF:Date:From:To:Subject:From:Subject; b=ZcfW3+qG7R79bk6QRJxEIQtkLCsE8bxHNrm/T1HWda+tAitSMd72JsVrmQGrENL1HSEly9kQZRwBLiU3GwGFOwBJuzGhve5VF1hBV4Lkqs61eVR3OMbalEe70NLPU+FOX2+Qh9KTIonV5cYl2J1zMW4CBc5o51RobfGEPWH5ykczdVHoaFPFr+GFqWwlMM12PDtEWC/Xj+vaYopN98jniIjS0RHwMd6HvMSFc4kpw4+kSDzkGE32/d6hFZUnbMgqwl/TqntLEyn9U8eXBwkDR7bN3jWGWyP1jRRmFLBpBtTyBlOs10nkK9t41lEvnpI87Wtw1udH2MzFsfB9RCN6Lg== X-YMail-OSG: q.ii6G4VM1kQgsul3WBtLLILgGKFENlk53oQI5pW6L91SfHCCcLnIBcC8Z4kDCj gsEjVuEUx_jfAfyl5QGAjokAKIZjy_abTzk7WTby7pT41mE4.RHR7ppoFyoJKMTQHKWdysyXNlqz Ea1tZJWdhrps4dxwIk53QBbn6oHD69P658WfXRTf_cO9w3KAxFgkHQ4onuBKpTzgPD0diHI69NJ5 YZD70H3IW.t7AQ1GsK2zKI6xmTsk8UlUjNLKKlZBfNDFpMSRQh_qnKVwAQ.iOTiM52vMI5VtP.A8 whBUTsaN0yNdvB1N6_aSeIYUr.Mtk6OeSmnIY4GOdceXwn8jj7inOUcVSBfgTr8JgoxZUdI1CsmR kXbHyZIqfh3JtJpR5nY6QBBqAKHB9fJlZOUxRxj8x6f6PJrrDmgUe1x3qhogRa3ltUJ0VSpKUlEL QA1hJC7iQRw6QA9wTgN4c4Zh8rXhZ0.ztAAdlpbKwMr3zY02LKMbqnuwp9ejkJKQVddNknyw_vfR rZMWbiVbvzJ0D5FkPiojdL9KJJcBO0hY_M8ThEgZ2GC.Mh5qjd_UfEK4GA2L9uHMJmEOGjcwo5WU idcDYZ5gffTNBwfNY.ylPygYo426.j1J0qCxceV9OgCmYRGl.d4nvsItIJOoAu_rqf77AFNRXsC5 lkSYIput.oABNX0GLEMvR331xjJZxEmHHMJufby4ZaO_ZHAKyj_Qbj6BTYCoeK9ctd00qVA0qrx9 6otDvqf1oUMNsNxtqA.SmtbpbvgEGdSVS0gQmKt3u1h00ygf22XdQ3YIg0lJz618NK4F.KjtTCVO 4SdmfPxxj7pfTek2eo4M_9QOGJfApnuxLXiKApiYEw X-Sonic-MF: X-Sonic-ID: e0120510-c654-4120-b386-4d12f705b579 Original-Received: from sonic.gate.mail.ne1.yahoo.com by sonic308.consmr.mail.bf2.yahoo.com with HTTP; Wed, 10 Apr 2024 15:54:20 +0000 Original-Received: by hermes--production-ir2-7bc88bfc75-7kc26 (Yahoo Inc. Hermes SMTP Server) with ESMTPA ID a4a7c7ed49fd81d1b90b548f166f93cf; Wed, 10 Apr 2024 15:54:17 +0000 (UTC) Content-Disposition: inline In-Reply-To: X-Mailer: WebService/1.1.22205 mail.backend.jedi.jws.acl:role.jedi.acl.token.atz.jws.hermes.aol Received-SPF: pass client-ip=74.6.130.41; envelope-from=spacibba@aol.com; helo=sonic308-2.consmr.mail.bf2.yahoo.com 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, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=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.29 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-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:317664 Archived-At: Hi: On Wed, Apr 10, 2024 at 01:55:36PM +0200, Eshel Yaron wrote: >Hi Ergus, > >Thank you for these comments. > >Ergus writes: > >> Hi: >> >> On Tue, Apr 09, 2024 at 08:30:16PM +0200, Eshel Yaron wrote: >> >> I don't think common preview it is less useful. Indeed what I find >> totally useless is the insertion of first candidate instead of the >> common prefix. Why the first? How far we need to rotate until we find >> the right one? isn't it faster just continue typing instead of >> next-next-next or insert the first and remove the last letters and >> re-write the ones I need? >> >> Inserting the common prefix is actually a good trick that asserts that >> the tab-insertion will be correct (but maybe incomplete), because when >> some candidate is shown it means that it is 100% sure what is valid >> after point in the whole completion list; but and also matches the >> muscular memory for bash users. > >I understand the value of inserting the common prefix as you describe >it. This is what the command completion-preview-insert-common-prefix >that I proposed up-thread does. It lets you insert (complete up to) the >longest common prefix even if the preview shows (also) the first suffix. > Indeed. The problem is that in my patch the user knows what's gonna be inserted. While otherwise we need to as some highlight to the candidate to let the user more or less know. To allow this second part the change implies modifications in all the functions chain to somehow export the prefix when shown + 1 extra command to insert only the common prefix + a user side rebind of tab. That change seems much more complicated (compared to my patch) Ideally We could add a approach to insert the common in the first tab (if available) and a the rest in a second tab... But again... this requires even more changes... not sure it worth it. >> In the minibuffer (and in bash and zsh) the behavior is >> similar. Generally we only need preview+tab-insert the common part, >> because the first candidate is not good in most of the cases; actually >> it is as good and bad than any other candidate (that's why packages that >> track and sort based on history, and context exist) OTOH to browse >> candidates is generally better things like corfu, company or >> auto-complete, so, a list/tooltip to navigate with extra information (a >> preview of 5-10 candidates, a counter of the total number, a counter >> with the relative position in the list, similar candidates grouped and >> sorted in the tooltip). >> >> Inserting the common part is useful when typing, to safe time, specially >> when working with long names prefixes packages (when all the functions >> and variables have the same prefix). Inserting the first candidate for >> those is actually annoying because there will be many candidates with >> same prefix, but different suffix. And rotating candidate is usually >> slower than just type some more letter. OTOH >> com-pr-somesufix is faster and intuitive than >> com.. specially because the user does not even >> know if the actual candidates list is long or short and how far the >> candidate may be. > >By using this completion-preview-insert-common-prefix (for example, by >binding it to TAB or another key in completion-preview-active-mode-map) >I think we should get that "com-pr-somesufix" behavior that >you're after. IIUC, the difference from what we get with the "common" >option in your patch is that in my approach the preview always shows a >full candidate, not just the longest common prefix. Right? > See above >> I don't mean that having a more elaborated preview is not useful; what I >> mean is that it doesn't worth it due to the code complexity and >> performance impact it generally implies. Many packages have tried this >> before. >> >> The package is useful as is, and in Emacs generally every new feature >> finds 100 requests for more and more details and configs hard to >> maintain and sadly mostly under-tested due to the high number of >> different config combinations needed. So I recommend KISS better and >> just add more features if they are actually requested by someone. >> >> But of course, you are free to implement the whole completion feature. >> Now you have the equivalents to company-preview-frontend and >> company-preview-if-just-one-frontend... my patch actually tried to mimic >> the missing company-preview-common-frontend > >Thanks, I'll take a look at company-preview-common-frontend as well. > >> If you allow me a couple of code comments: >> >> 1. Considering how the package works maybe it is useful to let-set: >> >> (completion-styles '(basic)) before calling completion-all-completions. >> >> Because you only care and handle the candidates where the matches are in >> the beginning, but with substring, partial-completion, etc the matches >> could be anywhere, giving a longer list (with many useless) >> candidates. > >I'll try it out and see if it makes for a significant performance >improvement. Otherwise, I think it's better not to override >completion-styles in case someone has some exotic completion style with >specific behavior that they rely on. IOW, I'd rather avoid making any >assumptions about completion-styles. > The assumption is actually good considering that your package handles candidates like (prefix)suffix inserting suffix; but it shouldn't handle correctly pre(prefix)suffix where `pre` is not inserted (or even previewed) If the user has some `flex` like style set by default it will just add even more noise; when what you care is just the prefix right? Also the try-completion function only handles correctly the candidates with the same prefix as it explains in the doc. So, in this case, the assumption is also a simplification, and consistency assertion with what the package is intended to do. >> 2. The `completion-all-completions`' output is a list with propertized >> string and information of the match location within the string... maybe >> it worth taking advantage of that information for the insertion or >> preview. > >Maybe, what do you have in mind? > The output already has fontified the matching part of the string... and the range in the match... you can just use that information as is to know what's the suffix. >> Otherwise it is probably better to use `all-completions` instead. It >> will be faster and you don't need to de-propertize the substrings to add >> your own fonts and properties. > >Note that we let-bind completion-lazy-hilit to t, so this is usually not >a problem. > > >Best, > >Eshel > Best, Ergus