From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?B?Sm/Do28gVMOhdm9yYQ==?= Newsgroups: gmane.emacs.devel Subject: Re: feature/icomplete-vertical Date: Mon, 05 Oct 2020 10:06:22 +0100 Message-ID: <87h7r9ghyp.fsf@gmail.com> References: <20200912133311.6ujtgczj6wyclufy@Ergus> <20200920130435.heye7bk73pm252km@Ergus> <83sgbczj0i.fsf@gnu.org> <83lfh4zfml.fsf@gnu.org> <838sd4z6lz.fsf@gnu.org> <20201001164804.mqqyxtet4ttweuyv@Ergus> <20201005044823.b2ougsrgcpvsuj74@Ergus> 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="31035"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Gregory Heytings , Eli Zaretskii , emacs-devel , Yuan Fu , Juri Linkov To: Ergus Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Oct 05 11:08:17 2020 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 1kPMTh-0007yJ-Dd for ged-emacs-devel@m.gmane-mx.org; Mon, 05 Oct 2020 11:08:17 +0200 Original-Received: from localhost ([::1]:54094 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kPMTg-0002sl-AN for ged-emacs-devel@m.gmane-mx.org; Mon, 05 Oct 2020 05:08:16 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46654) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kPMT2-0002Q0-CB for emacs-devel@gnu.org; Mon, 05 Oct 2020 05:07:36 -0400 Original-Received: from mail-wm1-x32b.google.com ([2a00:1450:4864:20::32b]:50414) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kPMSz-0004ti-Q1; Mon, 05 Oct 2020 05:07:35 -0400 Original-Received: by mail-wm1-x32b.google.com with SMTP id 13so7777454wmf.0; Mon, 05 Oct 2020 02:07:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=4Z6y9UmTmH6Iur5Ao3Eni6qcmEyx8SLcmwzTW/LH7a0=; b=g2irONLcerg/PCwEQ8aHe+XKiyusQqphFgTS7fifxdb2nZWwKMVb9NW1IcmZNIoYFs cjFNOzWi98Z3kZoR295WPUKqwTGQfIhJ/N4+0wFrZQVHwCT4mu//Q8/CwjgtQ3l0jaX0 +DDbvfln99cRMDQ7qc5ZZV7gnNaCxnR0sRxHRbJh3ni/UL/zRF7a8eDE7THwAXAVFsW7 IL9W7RHeVejuRTpECPJQV3rvZHEvWheNV+pIRfW+3NmhCA+2wtUzKJowuBKrF1MqXEcg 390b3pE0cAQtByDrIsD1oVqC8zj4ZZuk1xKcXbChQSEDBAAnpoPjPAz2UJMKoHJknD4r T6kA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=4Z6y9UmTmH6Iur5Ao3Eni6qcmEyx8SLcmwzTW/LH7a0=; b=El2hnJzrKkkLTuQDTIndk4rSNye9sVyEcKfHAuHrjWJcpZ4ydVJO78B4YG/K8as5uZ 4Q92c77HsYqEf+IfF3AFR66g+UmOa39wlTsgnps3VF1c2CX62jJSR8Tovi2JGA604HWD UrLEMtzVUbxsLboCJMnqodR8p3DqLG018x/ns5IRi5b+acHCTRtuAash+Zoz6JS42eDu k+teYcF9SUOHkquCULAkfsdXet2qxDvknaTNW2lGcqUDDrZBi/+MGlXqllNjKTrZ8O6L cj3mGZ/elEzxaHTs6e1lPNFdloBe3PS6wDxaX5wahoHozn2tQQyx/QUeuQnzGCMDg5UW X98A== X-Gm-Message-State: AOAM530osWAW+13naI0SqiPxjKjl4+55IMqacFiGUE/+ksauZFMxDCHu Mwq660wVKSLbNX3wHJxZJO8JuCeyWKI= X-Google-Smtp-Source: ABdhPJw98OQf35L7FnzIhMjneylZGBFuvPSMZbLQpF6l5qJ5+xk961UxlxoSF1skIhdhe7jSHEhnTQ== X-Received: by 2002:a1c:9c8c:: with SMTP id f134mr15832579wme.27.1601888851059; Mon, 05 Oct 2020 02:07:31 -0700 (PDT) Original-Received: from krug ([89.180.147.115]) by smtp.gmail.com with ESMTPSA id i16sm12402249wrq.73.2020.10.05.02.07.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Oct 2020 02:07:29 -0700 (PDT) In-Reply-To: <20201005044823.b2ougsrgcpvsuj74@Ergus> (Ergus's message of "Mon, 5 Oct 2020 06:48:23 +0200") Received-SPF: pass client-ip=2a00:1450:4864:20::32b; envelope-from=joaotavora@gmail.com; helo=mail-wm1-x32b.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. 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, 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.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:257090 Archived-At: Ergus writes: > On Mon, Oct 05, 2020 at 12:47:39AM +0100, Jo=C3=AF=C2=BF=C2=BDo T=C3=AF= =C2=BF=C2=BDvora wrote: > > Hi Joao: > > Please look at the previous messages in this same thread to see the > problems with your approach. > > So far there is: > > 1) The problem when using different font in minibuffer reported by > Jixiuf 2) the long line issue from Gregory (but also mentioned before) > 3) the ellipsis issue mentioned by Eli > 4) The prompt issue (which seems is not going to be solved as it is more > a design choice or a feature than a real issue) > 5) Respect the user customs max-mini-window-height and > resize-mini-windows so we shouldn't call enlarge-window and as an > core package we must not conflict with such customs. > 6) The removal of {} in vertical mode because in that case they are > redundant. > 7) The compatibility with packages like maple-minibuffer reported by > Dimitry. > 8) The case when using multiline separator. > 9) Move the first candidate to a newline instead of keeping it inline. > > The patch I propose is not complex at all considering all the above > conditions. The only complexity comes from the height calculation in > pixels which is something that will need to be done in any case either > for 1, 2, 3 or 4. Thanks for listing these again, for my benefit. I've not been following this very closely, and needed a summary to give me context after trying your patch again, as you requested. To be clear: I'm not "against" this functionality or even against it becoming a part of Emacs or Emacs core. I think solving the problems you list above is fine (though I haven't been by them, at least not yet) I'm not too much concerned about the complexity of implementation (such as the height calculation). I am concerned the changes to the user interface, of which your patch seems to bring many, or at least more than I would have expected. It seems to be that your solution mixes bugfixes and new features, which is what I object to. I think your solution could be trimmed down so that it solves _only_ the technical problems you list above without adding new user interfaces. In other words, don't see why the points above, which sound like basically bugs, need any new "defcustom". This is why I asked you to (re)list them. Then I can start with my naive implementation and fix them one by one, without adding new interfaces. Of course, then I will be repeating some work you've already done, so this is why I'm arguing for this separation in _your_ work. Could you structure your work so that one part fixes the above problems, and another part adds the new features, like customizing bracket types and such, and is should be discussed as a separate matter. After my sig, some comments that illustrate my points. I took this diff with git diff master...feature/icomplete-vertical Jo=C3=A3o > -(defcustom icomplete-separator " | " > - "String used by Icomplete to separate alternatives in the minibuffer." > - :type 'string > - :version "24.4") > +(defcustom icomplete-vertical nil > + "Enable `icomplete' vertical mode." > + :type 'bool > + :version "28.1") I would think it more elegant to keep `icomplete-separator` as the entry point to verticality. Certainly it should be possible to analyse the string and decide whether it implies verticality and needs special worries. (defcustom icomplete-hide-common-prefix t "When non-nil, hide common prefix from completion candidates. @@ -97,6 +97,12 @@ icomplete-first-match "Face used by Icomplete for highlighting first match." :version "24.4") =20 > +(defface icomplete-common-match '((t :inherit 'highlight > + :underline t > + :weight bold)) > + "Face used by Icomplete for highlighting common completion." > + :version "28.1") What is a "common match"? Is this exclusive to icomplete verticalness? Doesn't seem so, is this not a side feature? > -(defcustom icomplete-minibuffer-setup-hook nil > +(defvar icomplete-ellipsis nil) > + > +(defcustom icomplete--minibuffer-setup-hook nil > "Icomplete-specific customization of minibuffer setup. Why has this minibuffer hook been made an internal variable? > - (define-key map (kbd "") 'icomplete-forward-completions) > - (define-key map (kbd "") 'icomplete-backward-completions) Why have these been removed from icomplete-fido-mode-map? Also, I have C-r and C-r in that map, will it work in the verticality? I would hope it would, as it does not in my naive verticality implementation. > -;;;_ > icomplete-minibuffer-setup () > -(defun icomplete-minibuffer-setup () > +;; Vertical functions > + > +(defcustom icomplete-separator-vertical " \n" > + "String used by Icomplete to separate alternatives in the minibuffer." > + :type 'string > + :version "28.1") > + > +(defcustom icomplete-list-indicators-vertical (cons "" "") > + "Indicator bounds to list alternatives in the minibuffer." > + :type 'string > + :version "28.1") > + > +(defcustom icomplete-require-indicators-vertical (cons "" "") > + "Indicator bounds for match in the minibuffer when require-match." > + :type 'string > + :version "28.1") > + > +(defcustom icomplete-not-require-indicators-vertical (cons "" "") > + "Indicator bounds for match in the minibuffer when not require-match." > + :type 'string > + :version "28.1") These are examples of "interface bloat" that do not seem to directly address the 9 problems you listed in the beginning of the message (again, I am not against these features per se, mind you) > +(defvar icomplete--vertical-mode-map If this keymap is "internal" (as indicated by the two "--") how am I supposed to customize the keybindings? > +(defcustom icomplete-list-indicators-horizontal (cons "{" "}") > + "Indicator bounds to list alternatives in the minibuffer." > + :type 'string > + :version "28.1") > + > +(defcustom icomplete-require-indicators-horizontal (cons "(" ")") > + "Indicator bounds for match in the minibuffer when require-match." > + :type 'string > + :version "28.1") > + > +(defcustom icomplete-not-require-indicators-horizontal (cons "[" "]") > + "Indicator bounds for match in the minibuffer when not require-match." > + :type 'string > + :version "28.1") Again, these seem like improvements that are not directly related to verticality. (Maybe they are very good improvements, but still... Jo=C3=A3o