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: feature/icomplete-vertical Date: Sat, 19 Sep 2020 08:15:31 +0200 Message-ID: <20200919061531.oyjlbdvkbeif5fsg@Ergus> References: <20200919015957.prffuac2jke3hp6a@Ergus> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="neswyk7pmqxs2mjj" Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7566"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Eli Zaretskii , casouri@gmail.com, emacs-devel@gnu.org To: Gregory Heytings Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sat Sep 19 08:17:12 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 1kJWBL-0001r0-N7 for ged-emacs-devel@m.gmane-mx.org; Sat, 19 Sep 2020 08:17:11 +0200 Original-Received: from localhost ([::1]:49842 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kJWBK-0006Rq-Je for ged-emacs-devel@m.gmane-mx.org; Sat, 19 Sep 2020 02:17:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45060) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kJWA0-0005zS-GM for emacs-devel@gnu.org; Sat, 19 Sep 2020 02:15:48 -0400 Original-Received: from sonic312-22.consmr.mail.bf2.yahoo.com ([74.6.128.84]:38920) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kJW9w-0000Iw-Tj for emacs-devel@gnu.org; Sat, 19 Sep 2020 02:15:48 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aol.com; s=a2048; t=1600496142; bh=NmmFhCp+x+C8BQSFZvs2juYYpdmA0EOAYZpvP/ELlPM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From:Subject; b=aCfjcnh7zDqRbYhWIAbTAdMZkk6HFIP2TY79OzgAAtlshGreK04okVhtJPSoplyAUoL3na0gpmKhmGTUdxlclDylTNMHCrA29pVbs9MOiJtgFDVf/o6XepYYEM/rtsXoUGTWC+n7hEAbTFCbBlTI7rWwcLVNAPDn4XxvTS0stAMUKqhq9JscUYIgU6tU4T+wfXvqgn7NauQ4tWtqxao8YO0C5EqzcGxdh/AT65mNZIKHjRYz3V4w0fWCAuMIfyNyZUM+fqMD8+tksExcdgodlCS1NDd33jvGPrvU1OqyIsq/lh/LFEq1VuhQ++lHmuvAkATsjltRr6VxaTMbxKDoVQ== X-YMail-OSG: V0mfxg8VM1n7WcRaIfRT6YX1WysOwM_.xKK0DZRr2swY9UxZpAyW5JJ9ebeCp6j vvN7AXDds74MxD1CJHQRk5wWQaxUnzVcqHEdQC7Yimd5DKQRKuXfSk6de7IGw3NvV2sMerOnRDdy 1ndXYoWZWgIAoAY0IeyI7.h_H5hycNr0gZiDlchOCNrj12vcAFyBA_wpyrE43_9yQ.Pj9LAxEU8G GhyksuVI2CpGlEoXIgf94lViuiZ_Ngxaw_.mPw9_CcDFGnhY8sV48NOHAH6.MuBUVSKZG3USmf6E YZGJLNyxUJErYjp_agzYEybWhc981S6nRTslju5gpl2lO.uEcPxedp2G4UFAdCMQ2JallfYGNQ0J 8Er.RTa_.q3KRQgXSnlY2ihPKvpB1kgbUMt8_7oWz0enys_RWburCSFWWRwFXLdCShgJX4a5VHeh sVceA2on9T34_k0WRtysun5gZ_0AmvLcezVK5tx35ZGLT7uD7ScIBX.3CbK352vyXVCaIPVSyV6D GDxzjBNKiVI93L_jh4s0dEObAVLvEnh0DPg63E2wbNYEusYlSF55yyGkzX4ngIXSicY.FJe2DeHW KMlnqLca4afB5QXNu2czgvRkKcjaMVuDqz48sWFnq2Tf6qIBmQ2oh2X8sqlXT_GhXtlF_PQPI0_A ph9K8GPrp0_G._LomQddFdGbqaerlyAyu0RFEg3poyRYNTHGNVKFotLFPGfP15V46FLG03upk1IL ipWsxNo50HOjFKiysS4B2Pb8f786W4.H8a5tuh22yxhPx5b68LsYgG5E_V_SUO__.VU7OSnTjSge ptWTRiJGtEPNORy9lz2JLar7koV808WEgvZjLMM3Vw Original-Received: from sonic.gate.mail.ne1.yahoo.com by sonic312.consmr.mail.bf2.yahoo.com with HTTP; Sat, 19 Sep 2020 06:15:42 +0000 Original-Received: by smtp413.mail.ir2.yahoo.com (VZM Hermes SMTP Server) with ESMTPA ID ceae44cc54d1473183f3a1eb7b134d45; Sat, 19 Sep 2020 06:15:40 +0000 (UTC) Content-Disposition: inline In-Reply-To: X-Mailer: WebService/1.1.16583 mail.backend.jedi.jws.acl:role.jedi.acl.token.atz.jws.hermes.aol Apache-HttpAsyncClient/4.1.4 (Java/11.0.7) Received-SPF: pass client-ip=74.6.128.84; envelope-from=spacibba@aol.com; helo=sonic312-22.consmr.mail.bf2.yahoo.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/19 02:15:42 X-ACL-Warn: Detected OS = Linux 3.11 and newer [fuzzy] 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=unavailable 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:256146 Archived-At: --neswyk7pmqxs2mjj Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline On Sat, Sep 19, 2020 at 04:03:36AM +0000, Gregory Heytings wrote: > >Hi Ergus, > >Thanks for your comments. > > >Okay, these lines in my patch can just be dropped indeed. This makes >the whole thing even simpler. > I thought the same on the beginning, but then the sizes issues came here and there. > >I see. I did not think about that potential problem before sending >the patch. I attach an updated patch (still very short) in which this >problem is (I think) fixed. > There is another corner case that is when (frame-parameter nil 'minibuffer) is 'only. This was also a problem reported some days ago needed when using frames and not the minibuffer. > >That could be an additional feature indeed. I do not understand why >they are annoying, it's common in completion mechanisms to have such >indications, but why not. But I think it is better to separate >concerns. > The point is that they process the output and substitute the [] () {} and | with an advise. Not only in the external package but also some custom configurations they sent by mail made many magics with this. And it was explicitly requested by a user some days ago. > >That could also be an additional feature, again I don't really see why >it would be useful, but why not. OTOH, it is not possible to do this >with the current version of icomplete in horizontal mode, and again I >think it is better to separate concerns. > With the advise it is possible and more or less simple. That's why I have been playing with the formats to see if I can provide that with a simple code. But ofcourse, it is expected to be an additional feature. > >The patch I propose has only five "if icomplete-vertical", that's not >much I think. And in fact it's easy to remove three of them, and to >have only two "if icomplete-vertical" (see the other attached patch). >If horizontal icomplete remains the default behavior (as I think it >will), these if's make it clear where the code differs between the two >cases. > The other problem with the patch is that due to rounding and floor when using different fonts there is too much extra space missing and sometimes missed a candidate at the end. This was also reported some days ago. >Gregory Look at the attached patch as it is partially simplified in my local branch. It can be still very improved (the separator part, for example needs some extra checks to assert it contains at least a '\n', I am actually thinking in removing the icomplete-format variable and just look for a \n in the separator in the setup; it will be simpler indeed) but also some actions are repeated with no reason. But at least it includes the minimal changes, the corner cases with the windows/frames/fonts size, fixes the prompt hidden issue, respects the user separator variable and reduces the conditions we check to only 1 in the setup. Best, Ergus. --neswyk7pmqxs2mjj Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename="icomplete-vertical.patch" Content-Transfer-Encoding: 8bit diff --git a/lisp/icomplete.el b/lisp/icomplete.el index 4e546807b7..c1028e84d0 100644 --- a/lisp/icomplete.el +++ b/lisp/icomplete.el @@ -50,6 +50,7 @@ ;;; Code: (require 'rfn-eshadow) ; rfn-eshadow-overlay +(require 'cl-lib) (defgroup icomplete nil "Show completions dynamically in minibuffer." @@ -57,7 +58,7 @@ icomplete :link '(info-link "(emacs)Icomplete") :group 'minibuffer) -(defcustom icomplete-separator " | " +(defcustom icomplete-separator nil "String used by Icomplete to separate alternatives in the minibuffer." :type 'string :version "24.4") @@ -68,6 +69,12 @@ icomplete-hide-common-prefix :type 'boolean :version "24.4") +(defcustom icomplete-format 'horizontal + "Enable `icomplete' vertical mode." + :type '(choice (const horizontal) + (const vertical)) + :version "28.1") + (defvar icomplete-tidy-shadowed-file-names nil "If non-nil, automatically delete superfluous parts of file names. For example, if the user types ~/ after a long path name, @@ -139,6 +146,22 @@ icomplete-minibuffer-setup-hook :group 'icomplete) +(defvar-local icomplete--ellipsis nil + "Ellipsis symbol to indicate continuation.") + +(defvar-local icomplete--separator nil + "If there are multiple possibilities this separates them.") + +(defvar-local icomplete--list-indicators-format nil + "Indicator for when multiple prospects are available. +means that further input is required to distinguish a single one") + +(defvar-local icomplete--last-format nil) +(defvar-local icomplete--prospects nil) +(defvar-local icomplete--match-braket nil) +(defvar-local icomplete--prospects-max-pixel-height nil) + + ;;;_* Initialization ;;;_ + Internal Variables @@ -437,6 +460,61 @@ icomplete-simple-completing-p (member table icomplete-with-completion-tables)))))) ;;;_ > icomplete-minibuffer-setup () + +(defun icomplete--vertical-prospects (prefix-len _determ comps) + "List of vertical completions limited." + ;; Max total rows to use, including the minibuffer content. + (let* ((single-line-height (line-pixel-height)) + (line-height (* (cl-count ?\n icomplete--separator) single-line-height)) + ;; Needed for prospects-max-height-pixel + (prospects-rows-pixel (* (+ 1 + (cl-count ?\n icomplete--match-braket) + (cl-count ?\n icomplete--list-indicators-format)) + single-line-height)) + (prospects-max-pixel-height (min (+ prospects-rows-pixel + (* icomplete-prospects-height line-height)) + icomplete--prospects-max-pixel-height)) + limit prospects comp) + + + (while (and comps (not limit)) + (setq comp (substring (pop comps) prefix-len) + prospects-rows-pixel (+ prospects-rows-pixel line-height)) + + (if (< prospects-rows-pixel prospects-max-pixel-height) + (push comp prospects) + (push icomplete--ellipsis prospects) + (setq limit t))) + (nreverse prospects))) + + +(defun icomplete--horizontal-prospects (prefix-len determ comps) + "List of horizontal completions limited." + + (let* (;; Max total length to use, including the minibuffer content. + (separator-width (string-width icomplete--separator)) + (prospects-len (+ (string-width (or determ (format icomplete--match-braket ""))) + (string-width icomplete--separator) + (+ 2 (string-width icomplete--ellipsis)) ;; take {…} into account + (string-width (buffer-string)))) + (prospects-max-len (* (+ icomplete-prospects-height + ;; If the minibuffer content already uses up more than + ;; one line, increase the allowable space accordingly. + (/ prospects-len (window-width))) + (window-width))) + limit prospects comp) + + (while (and comps (not limit)) + (setq comp (substring (pop comps) prefix-len) + prospects-len (+ prospects-len (string-width comp) separator-width)) + + (if (< prospects-len prospects-max-len) + (push comp prospects) + (push icomplete--ellipsis prospects) + (setq limit t))) + (nreverse prospects))) + + (defun icomplete-minibuffer-setup () "Run in minibuffer on activation to establish incremental completion. Usually run by inclusion in `minibuffer-setup-hook'." @@ -446,7 +524,31 @@ icomplete-minibuffer-setup (current-local-map))) (add-hook 'pre-command-hook #'icomplete-pre-command-hook nil t) (add-hook 'post-command-hook #'icomplete-post-command-hook nil t) - (run-hooks 'icomplete-minibuffer-setup-hook))) + (run-hooks 'icomplete-minibuffer-setup-hook) + + (setq-local icomplete--ellipsis (if (char-displayable-p ?…) "…" "...")) + + (cond + ((eq icomplete-format 'vertical) + (setq-local icomplete--list-indicators-format " \n%s" + icomplete--separator (or icomplete-separator "\n") + icomplete--prospects 'icomplete--vertical-prospects + icomplete--prospects-max-pixel-height + (let ((minibuffer-parameter (frame-parameter nil 'minibuffer))) + (cond + ((eq minibuffer-parameter t) + (cond ((floatp max-mini-window-height) + (floor (* max-mini-window-height (frame-pixel-height)))) + ((integerp max-mini-window-height) + (floor (* max-mini-window-height (frame-char-height)))) + (t + (* icomplete-prospects-height (frame-char-height))))) + ((eq minibuffer-parameter 'only) + (frame-pixel-height)))))) + (t (setq-local icomplete--list-indicators-format "{%s}" + icomplete--separator (or icomplete-separator " | ") + icomplete--prospects 'icomplete--horizontal-prospects))))) + (defvar icomplete--in-region-buffer nil) @@ -639,8 +741,6 @@ icomplete-completions (...) - a single prospect is identified and matching is enforced, [...] - a single prospect is identified but matching is optional, or - {...} - multiple prospects, separated by commas, are indicated, and - further input is required to distinguish a single one. If there are multiple possibilities, `icomplete-separator' separates them. @@ -665,13 +765,13 @@ icomplete-completions (md (completion--field-metadata (icomplete--field-beg))) (comps (icomplete--sorted-completions)) (last (if (consp comps) (last comps))) - (base-size (cdr last)) - (open-bracket (if require-match "(" "[")) - (close-bracket (if require-match ")" "]"))) + (base-size (cdr last))) + + (setq-local icomplete--match-braket (if require-match "(%s)" "[%s]")) ;; `concat'/`mapconcat' is the slow part. (if (not (consp comps)) (progn ;;(debug (format "Candidates=%S field=%S" candidates name)) - (format " %sNo matches%s" open-bracket close-bracket)) + (format icomplete--match-braket "No matches")) (if last (setcdr last nil)) (let* ((most-try (if (and base-size (> base-size 0)) @@ -687,33 +787,18 @@ icomplete-completions ;; a prefix of most, or something else. (compare (compare-strings name nil nil most nil nil completion-ignore-case)) - (ellipsis (if (char-displayable-p ?…) "…" "...")) (determ (unless (or (eq t compare) (eq t most-try) (= (setq compare (1- (abs compare))) (length most))) - (concat open-bracket + (format icomplete--match-braket (cond ((= compare (length name)) ;; Typical case: name is a prefix. (substring most compare)) ;; Don't bother truncating if it doesn't gain ;; us at least 2 columns. - ((< compare (+ 2 (string-width ellipsis))) most) - (t (concat ellipsis (substring most compare)))) - close-bracket))) - ;;"-prospects" - more than one candidate - (prospects-len (+ (string-width - (or determ (concat open-bracket close-bracket))) - (string-width icomplete-separator) - (+ 2 (string-width ellipsis)) ;; take {…} into account - (string-width (buffer-string)))) - (prospects-max - ;; Max total length to use, including the minibuffer content. - (* (+ icomplete-prospects-height - ;; If the minibuffer content already uses up more than - ;; one line, increase the allowable space accordingly. - (/ prospects-len (window-width))) - (window-width))) + ((< compare (+ 2 (string-width icomplete--ellipsis))) most) + (t (concat icomplete--ellipsis (substring most compare))))))) ;; Find the common prefix among `comps'. ;; We can't use the optimization below because its assumptions ;; aren't always true, e.g. when completion-cycling (bug#10850): @@ -725,12 +810,13 @@ icomplete-completions (prefix (when icomplete-hide-common-prefix (try-completion "" comps))) (prefix-len - (and (stringp prefix) + (and icomplete-hide-common-prefix + (stringp prefix) ;; Only hide the prefix if the corresponding info ;; is already displayed via `most'. (string-prefix-p prefix most t) - (length prefix))) ;;) - prospects comp limit) + (length prefix))) + prospects) (if (or (eq most-try t) (not (consp (cdr comps)))) (setq prospects nil) (when (member name comps) @@ -751,20 +837,11 @@ icomplete-completions ;; To circumvent all the above problems, provide a visual ;; cue to the user via an "empty string" in the try ;; completion field. - (setq determ (concat open-bracket "" close-bracket))) + (setq determ (format icomplete--match-braket ""))) ;; Compute prospects for display. - (while (and comps (not limit)) - (setq comp - (if prefix-len (substring (car comps) prefix-len) (car comps)) - comps (cdr comps)) - (setq prospects-len - (+ (string-width comp) - (string-width icomplete-separator) - prospects-len)) - (if (< prospects-len prospects-max) - (push comp prospects) - (setq limit t)))) - (setq prospects (nreverse prospects)) + (setq prospects + (funcall icomplete--prospects prefix-len determ comps))) + ;; Decorate first of the prospects. (when prospects (let ((first (copy-sequence (pop prospects)))) @@ -776,10 +853,8 @@ icomplete-completions (if last (setcdr last base-size)) (if prospects (concat determ - "{" - (mapconcat 'identity prospects icomplete-separator) - (and limit (concat icomplete-separator ellipsis)) - "}") + (format icomplete--list-indicators-format + (mapconcat 'identity prospects icomplete--separator))) (concat determ " [Matched]")))))) ;;; Iswitchb compatibility --neswyk7pmqxs2mjj--