* bug#59486: completion-auto-wrap disobeyed by vertical navigation @ 2022-11-22 17:38 Juri Linkov 2022-11-23 18:49 ` Juri Linkov 0 siblings, 1 reply; 15+ messages in thread From: Juri Linkov @ 2022-11-22 17:38 UTC (permalink / raw) To: 59486 [-- Attachment #1: Type: text/plain, Size: 219 bytes --] In a multi-column layout, the keys <left> and <right> wrap to the beginning/end of the completions buffer, but <up> and <down> don't. Here is a patch that supports completion-auto-wrap for wrapping to the top/bottom: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: next-line-completion.patch --] [-- Type: text/x-diff, Size: 3612 bytes --] diff --git a/lisp/simple.el b/lisp/simple.el index 0f44b14948c..459af2fcf55 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -9572,6 +9574,8 @@ completion-list-mode-map (define-key map "\C-m" 'choose-completion) (define-key map "\e\e\e" 'delete-completion-window) (define-key map [remap keyboard-quit] #'delete-completion-window) + (define-key map [up] 'previous-line-completion) + (define-key map [down] 'next-line-completion) (define-key map [left] 'previous-completion) (define-key map [right] 'next-completion) (define-key map [?\t] 'next-completion) @@ -9736,6 +9740,77 @@ next-completion (when (/= 0 n) (switch-to-minibuffer)))) +(defun previous-line-completion (&optional n) + "Move to the item on the previous line in the completion list. +With prefix argument N, move back N items line-wise (negative N +means move forward). + +Also see the `completion-auto-wrap' variable." + (interactive "p") + (next-line-completion (- n))) + +(defun next-line-completion (&optional n) + "Move to the item on the next line in the completion list. +With prefix argument N, move N items line-wise (negative N +means move backward). + +Also see the `completion-auto-wrap' variable." + (interactive "p") + (let ((column (current-column)) + pos) + (catch 'bound + (while (> n 0) + (setq pos nil) + (save-excursion + (while (and (not pos) (not (eobp))) + (forward-line 1) + (when (and (not (eobp)) + (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))))) + (if pos + (goto-char pos) + (when completion-auto-wrap + (let ((column (current-column))) + (save-excursion + (goto-char (point-min)) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))) + (while (and (not pos) (not (eobp))) + (forward-line 1) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))))) + (if pos (goto-char pos))))) + (setq n (1- n))) + + (while (< n 0) + (setq pos nil) + (save-excursion + (while (and (not pos) (not (bobp))) + (forward-line -1) + (when (and (not (bobp)) + (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))))) + (if pos + (goto-char pos) + (when completion-auto-wrap + (let ((column (current-column))) + (save-excursion + (goto-char (point-max)) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))) + (while (and (not pos) (not (bobp))) + (forward-line -1) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))))) + (if pos (goto-char pos))))) + (setq n (1+ n)))))) + (defun choose-completion (&optional event no-exit no-quit) "Choose the completion at point. If EVENT, use EVENT's position to determine the starting position. ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-22 17:38 bug#59486: completion-auto-wrap disobeyed by vertical navigation Juri Linkov @ 2022-11-23 18:49 ` Juri Linkov 2022-11-24 7:59 ` Juri Linkov 2023-10-31 7:44 ` Juri Linkov 0 siblings, 2 replies; 15+ messages in thread From: Juri Linkov @ 2022-11-23 18:49 UTC (permalink / raw) To: 59486 [-- Attachment #1: Type: text/plain, Size: 340 bytes --] > In a multi-column layout, the keys <left> and <right> > wrap to the beginning/end of the completions buffer, > but <up> and <down> don't. Here is a patch that supports > completion-auto-wrap for wrapping to the top/bottom: Now pushed. And here are the corresponding commands for navigating the completions buffer from the minibuffer: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: minibuffer-next-completion.patch --] [-- Type: text/x-diff, Size: 1914 bytes --] diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 6bb0fa3ae98..ea1e88c7234 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4452,7 +4456,7 @@ minibuffer-completion-auto-choose :type 'boolean :version "29.1") -(defun minibuffer-next-completion (&optional n) +(defun minibuffer-next-completion (&optional n vertical) "Move to the next item in its completions window from the minibuffer. When `minibuffer-completion-auto-choose' is non-nil, then also insert the selected completion to the minibuffer." @@ -4461,7 +4465,9 @@ minibuffer-next-completion (with-minibuffer-completions-window (when completions-highlight-face (setq-local cursor-face-highlight-nonselected-window t)) - (next-completion (or n 1)) + (if vertical + (next-line-completion (or n 1)) + (next-completion (or n 1))) (when auto-choose (let ((completion-use-base-affixes t)) (choose-completion nil t t)))))) @@ -4473,6 +4479,20 @@ minibuffer-previous-completion (interactive "p") (minibuffer-next-completion (- (or n 1)))) +(defun minibuffer-next-line-completion (&optional n) + "Move to the next completion line from the minibuffer. +When `minibuffer-completion-auto-choose' is non-nil, then also +insert the selected completion to the minibuffer." + (interactive "p") + (minibuffer-next-completion (or n 1) t)) + +(defun minibuffer-previous-line-completion (&optional n) + "Move to the previous completion line from the minibuffer. +When `minibuffer-completion-auto-choose' is non-nil, then also +insert the selected completion to the minibuffer." + (interactive "p") + (minibuffer-next-completion (- (or n 1)) t)) + (defun minibuffer-choose-completion (&optional no-exit no-quit) "Run `choose-completion' from the minibuffer in its completions window. With prefix argument NO-EXIT, insert the completion at point to the ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-23 18:49 ` Juri Linkov @ 2022-11-24 7:59 ` Juri Linkov 2022-11-24 8:13 ` Eli Zaretskii 2023-10-31 7:44 ` Juri Linkov 1 sibling, 1 reply; 15+ messages in thread From: Juri Linkov @ 2022-11-24 7:59 UTC (permalink / raw) To: 59486 > And here are the corresponding commands for navigating > the completions buffer from the minibuffer: > > +(defun minibuffer-next-line-completion (&optional n) > +(defun minibuffer-previous-line-completion (&optional n) It seems there commands can't be bound in the minibuffer. Ideally, these keybindings could be added to minibuffer-local-completion-map: "M-<left>" #'minibuffer-previous-completion "M-<right>" #'minibuffer-next-completion "M-<up>" #'minibuffer-previous-line-completion "M-<down>" #'minibuffer-next-line-completion But maybe 'M-<left>' and 'M-<right>' can't be taken from moving by words even when 'C-<left>' and 'C-<right>' are duplicate keys that are doing the same. Or to bind 'M-<left>' and 'M-<right>' only in completion-in-region-mode-map that is more transient by nature and is active only as long as the completions buffer is shown. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-24 7:59 ` Juri Linkov @ 2022-11-24 8:13 ` Eli Zaretskii 2022-11-24 18:30 ` Juri Linkov 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2022-11-24 8:13 UTC (permalink / raw) To: Juri Linkov; +Cc: 59486 > From: Juri Linkov <juri@linkov.net> > Date: Thu, 24 Nov 2022 09:59:52 +0200 > > > And here are the corresponding commands for navigating > > the completions buffer from the minibuffer: > > > > +(defun minibuffer-next-line-completion (&optional n) > > +(defun minibuffer-previous-line-completion (&optional n) > > It seems there commands can't be bound in the minibuffer. > Ideally, these keybindings could be added to > minibuffer-local-completion-map: > > "M-<left>" #'minibuffer-previous-completion > "M-<right>" #'minibuffer-next-completion > "M-<up>" #'minibuffer-previous-line-completion > "M-<down>" #'minibuffer-next-line-completion > > But maybe 'M-<left>' and 'M-<right>' can't be taken from moving by words > even when 'C-<left>' and 'C-<right>' are duplicate keys that are doing > the same. Or to bind 'M-<left>' and 'M-<right>' only in > completion-in-region-mode-map that is more transient by nature > and is active only as long as the completions buffer is shown. Please don't usurp M-<LEFT> and M-<RIGHT>, as they are needed on TTY frames where C-<LEFT> and C-<RIGHT> are not available. I don't really understand the difference between minibuffer-previous-* and minibuffer-previous-line-* commands (the available documentation is minimal and doesn't explain this difference), so it's hard to suggest an alternative. But one immediate alternative is to use <LEFT> and <RIGHT> for the minibuffer-previous-* family, like previous-completion does. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-24 8:13 ` Eli Zaretskii @ 2022-11-24 18:30 ` Juri Linkov 2022-11-24 18:43 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Juri Linkov @ 2022-11-24 18:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 59486 > I don't really understand the difference between minibuffer-previous-* and > minibuffer-previous-line-* commands (the available documentation is minimal > and doesn't explain this difference), so it's hard to suggest an alternative. minibuffer-previous-* are intended to navigate the completion list horizontally while the minibuffer is active, and minibuffer-previous-line-* vertically. > But one immediate alternative is to use <LEFT> and <RIGHT> for > the minibuffer-previous-* family, like previous-completion does. <LEFT> and <RIGHT> are useful to move point in the minibuffer. But do you think it would be okay to use <LEFT> and <RIGHT> for navigation of completions from the minibuffer after adding a new user option, when is non-nil (disabled by default)? ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-24 18:30 ` Juri Linkov @ 2022-11-24 18:43 ` Eli Zaretskii 2022-11-25 7:47 ` Juri Linkov 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2022-11-24 18:43 UTC (permalink / raw) To: Juri Linkov; +Cc: 59486 > From: Juri Linkov <juri@linkov.net> > Cc: 59486@debbugs.gnu.org > Date: Thu, 24 Nov 2022 20:30:00 +0200 > > > I don't really understand the difference between minibuffer-previous-* and > > minibuffer-previous-line-* commands (the available documentation is minimal > > and doesn't explain this difference), so it's hard to suggest an alternative. > > minibuffer-previous-* are intended to navigate the completion list > horizontally while the minibuffer is active, and minibuffer-previous-line-* > vertically. Then the names could be simplified as minibuffer-up-line and minibuffer-down-line. It will also make the names less confusing, IMO. > > But one immediate alternative is to use <LEFT> and <RIGHT> for > > the minibuffer-previous-* family, like previous-completion does. > > <LEFT> and <RIGHT> are useful to move point in the minibuffer. Then maybe PgUp and PgDn? > But do you think it would be okay to use <LEFT> and <RIGHT> > for navigation of completions from the minibuffer after > adding a new user option, when is non-nil (disabled by default)? If the arrow keys are already taken, having an option that will change their effect would be confusing, I think. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-24 18:43 ` Eli Zaretskii @ 2022-11-25 7:47 ` Juri Linkov 2022-11-25 8:38 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Juri Linkov @ 2022-11-25 7:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 59486 >> minibuffer-previous-* are intended to navigate the completion list >> horizontally while the minibuffer is active, and minibuffer-previous-line-* >> vertically. > > Then the names could be simplified as minibuffer-up-line and > minibuffer-down-line. It will also make the names less confusing, IMO. It's not only about the minibuffer, but also about completions, so the commands names should also include the word "completion". >> > But one immediate alternative is to use <LEFT> and <RIGHT> for >> > the minibuffer-previous-* family, like previous-completion does. >> >> <LEFT> and <RIGHT> are useful to move point in the minibuffer. > > Then maybe PgUp and PgDn? PgUp and PgDn are used to scroll the completions buffer from the minibuffer. >> But do you think it would be okay to use <LEFT> and <RIGHT> >> for navigation of completions from the minibuffer after >> adding a new user option, when is non-nil (disabled by default)? > > If the arrow keys are already taken, having an option that will change their > effect would be confusing, I think. It does the same as icomplete-vertical-mode where the arrow keys are not confusing. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-25 7:47 ` Juri Linkov @ 2022-11-25 8:38 ` Eli Zaretskii 2022-11-28 7:56 ` Juri Linkov 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2022-11-25 8:38 UTC (permalink / raw) To: Juri Linkov; +Cc: 59486 > From: Juri Linkov <juri@linkov.net> > Cc: 59486@debbugs.gnu.org > Date: Fri, 25 Nov 2022 09:47:29 +0200 > > >> minibuffer-previous-* are intended to navigate the completion list > >> horizontally while the minibuffer is active, and minibuffer-previous-line-* > >> vertically. > > > > Then the names could be simplified as minibuffer-up-line and > > minibuffer-down-line. It will also make the names less confusing, IMO. > > It's not only about the minibuffer, but also about completions, > so the commands names should also include the word "completion". minibuffer-up-completions-line is still shorter and less confusing. > >> > But one immediate alternative is to use <LEFT> and <RIGHT> for > >> > the minibuffer-previous-* family, like previous-completion does. > >> > >> <LEFT> and <RIGHT> are useful to move point in the minibuffer. > > > > Then maybe PgUp and PgDn? > > PgUp and PgDn are used to scroll the completions buffer from the minibuffer. That's a bad idea, IMO: scrolling another window should use something like M-PgDn and C-M-v, like we do with other windows elsewhere. > > If the arrow keys are already taken, having an option that will change their > > effect would be confusing, I think. > > It does the same as icomplete-vertical-mode where the arrow keys > are not confusing. icomplete-vertical-mode is a an opt-in feature, so what it does is less relevant to the issue at point, IMO. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-25 8:38 ` Eli Zaretskii @ 2022-11-28 7:56 ` Juri Linkov 0 siblings, 0 replies; 15+ messages in thread From: Juri Linkov @ 2022-11-28 7:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 59486 >> >> minibuffer-previous-* are intended to navigate the completion list >> >> horizontally while the minibuffer is active, and minibuffer-previous-line-* >> >> vertically. >> > >> > Then the names could be simplified as minibuffer-up-line and >> > minibuffer-down-line. It will also make the names less confusing, IMO. >> >> It's not only about the minibuffer, but also about completions, >> so the commands names should also include the word "completion". > > minibuffer-up-completions-line is still shorter and less confusing. 'minibuffer-previous-line-completion' is named after 'previous-line' that is not named 'up-line'. There are 'left-char', 'left-word', etc. only in horizontal direction, but no 'up-line' in vertical direction. So it makes no sense to propagate this inconsistency to completion command names where there are only 'previous-completion', but no 'left-completion'. >> It does the same as icomplete-vertical-mode where the arrow keys >> are not confusing. > > icomplete-vertical-mode is a an opt-in feature, so what it does is less > relevant to the issue at point, IMO. The proposed feature is also opt-in. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2022-11-23 18:49 ` Juri Linkov 2022-11-24 7:59 ` Juri Linkov @ 2023-10-31 7:44 ` Juri Linkov 2023-11-01 17:45 ` Juri Linkov 1 sibling, 1 reply; 15+ messages in thread From: Juri Linkov @ 2023-10-31 7:44 UTC (permalink / raw) To: 59486 [-- Attachment #1: Type: text/plain, Size: 256 bytes --] > In a multi-column layout, the keys <left> and <right> > wrap to the beginning/end of the completions buffer, > but <up> and <down> don't. Here is a patch that supports > completion-auto-wrap for wrapping to the top/bottom: Ok, here is a better patch: [-- Attachment #2: next-line-completion.patch --] [-- Type: text/x-diff, Size: 5691 bytes --] diff --git a/etc/NEWS b/etc/NEWS index 9c0f28e3fa9..cabdfe50a19 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -999,8 +999,15 @@ A major mode based on the tree-sitter library for editing Elixir files. *** New major mode 'lua-ts-mode'. A major mode based on the tree-sitter library for editing Lua files. +** Minibuffer and Completions + +*** New commands 'previous-line-completion' and 'next-line-completion'. +Bound to [up] and [down] respectively, they navigate the *Completions* +buffer vertically, wrapping at the top/bottom when 'completion-auto-wrap' +is non-nil. + +++ -** New global minor mode 'minibuffer-regexp-mode'. +*** New global minor mode 'minibuffer-regexp-mode'. This is a minor mode for editing regular expressions in the minibuffer. It highlights parens via ‘show-paren-mode’ and ‘blink-matching-paren’ in a user-friendly way, avoids reporting alleged paren mismatches and makes diff --git a/lisp/simple.el b/lisp/simple.el index ec14bec9e07..b0782d1f8ae 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -9820,6 +9827,8 @@ completion-list-mode-map (define-key map "\C-m" 'choose-completion) (define-key map "\e\e\e" 'delete-completion-window) (define-key map [remap keyboard-quit] #'delete-completion-window) + (define-key map [up] 'previous-line-completion) + (define-key map [down] 'next-line-completion) (define-key map [left] 'previous-completion) (define-key map [right] 'next-completion) (define-key map [?\t] 'next-completion) @@ -9882,7 +9891,8 @@ delete-completion-window (defcustom completion-auto-wrap t "Non-nil means to wrap around when selecting completion options. -This affects the commands `next-completion' and `previous-completion'. +This affects the commands `next-completion', `previous-completion', +`next-line-completion' and `previous-line-completion'. When `completion-auto-select' is t, it wraps through the minibuffer for the commands bound to the TAB key." :type 'boolean @@ -10000,6 +10010,87 @@ next-completion (when (/= 0 n) (switch-to-minibuffer)))) +(defun previous-line-completion (&optional n) + "Move to the item on the previous line in the completion list. +With prefix argument N, move back N items line-wise (negative N +means move forward). + +Also see the `completion-auto-wrap' variable." + (interactive "p") + (next-line-completion (- n))) + +(defun next-line-completion (&optional n) + "Move to the item on the next line in the completion list. +With prefix argument N, move N items line-wise (negative N +means move backward). + +Also see the `completion-auto-wrap' variable." + (interactive "p") + (let (line column pos) + (when (and (bobp) + (> n 0) + (get-text-property (point) 'mouse-face) + (not (get-text-property (point) 'first-completion))) + (let ((inhibit-read-only t)) + (add-text-properties (point) (1+ (point)) '(first-completion t))) + (setq n (1- n))) + + (if (get-text-property (point) 'mouse-face) + ;; If in a completion, move to the start of it. + (when (and (not (bobp)) + (get-text-property (1- (point)) 'mouse-face)) + (goto-char (previous-single-property-change (point) 'mouse-face))) + ;; Try to move to the previous completion. + (setq pos (previous-single-property-change (point) 'mouse-face)) + (if pos + ;; Move to the start of the previous completion. + (progn + (goto-char pos) + (unless (get-text-property (point) 'mouse-face) + (goto-char (previous-single-property-change + (point) 'mouse-face nil (point-min))))) + (cond ((> n 0) (setq n (1- n)) (first-completion)) + ((< n 0) (first-completion))))) + + (while (> n 0) + (setq pos nil column (current-column) line (line-number-at-pos)) + (when (and (or (not (eq (forward-line 1) 0)) + (eobp) + (not (eq (move-to-column column) column)) + (not (get-text-property (point) 'mouse-face))) + completion-auto-wrap) + (save-excursion + (goto-char (point-min)) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))) + (while (and (not pos) (> line (line-number-at-pos))) + (forward-line 1) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))))) + (if pos (goto-char pos))) + (setq n (1- n))) + + (while (< n 0) + (setq pos nil column (current-column) line (line-number-at-pos)) + (when (and (or (not (eq (forward-line -1) 0)) + (not (eq (move-to-column column) column)) + (not (get-text-property (point) 'mouse-face))) + completion-auto-wrap) + (save-excursion + (goto-char (point-max)) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))) + (while (and (not pos) (< line (line-number-at-pos))) + (forward-line -1) + (when (and (eq (move-to-column column) column) + (get-text-property (point) 'mouse-face)) + (setq pos (point))))) + (if pos (goto-char pos))) + (setq n (1+ n))))) + (defun choose-completion (&optional event no-exit no-quit) "Choose the completion at point. If EVENT, use EVENT's position to determine the starting position. ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2023-10-31 7:44 ` Juri Linkov @ 2023-11-01 17:45 ` Juri Linkov 2023-11-02 8:11 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Juri Linkov @ 2023-11-01 17:45 UTC (permalink / raw) To: 59486 [-- Attachment #1: Type: text/plain, Size: 608 bytes --] >> In a multi-column layout, the keys <left> and <right> >> wrap to the beginning/end of the completions buffer, >> but <up> and <down> don't. Here is a patch that supports >> completion-auto-wrap for wrapping to the top/bottom: > > Ok, here is a better patch: Pushed to master. Now here is a patch that implements the long-discussed feature of using arrow keys for navigating completions from the minibuffer only when the *Completions* buffer is visible. It's disabled by default and can be enabled by the option 'minibuffer-completion-visible'. It works nicely for the in-buffer completions as well: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: minibuffer-completion-visible.patch --] [-- Type: text/x-diff, Size: 4743 bytes --] diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 2120e31775e..2601e705aa0 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -2707,8 +2716,14 @@ completion-in-region-mode completion-in-region-mode-predicate) (setq-local minibuffer-completion-auto-choose nil) (add-hook 'post-command-hook #'completion-in-region--postch) - (push `(completion-in-region-mode . ,completion-in-region-mode-map) - minor-mode-overriding-map-alist))) + (let* ((keymap completion-in-region-mode-map) + (keymap (if minibuffer-completion-visible + (make-composed-keymap + (list minibuffer-local-visible-completion-map + keymap)) + keymap))) + (push `(completion-in-region-mode . ,keymap) + minor-mode-overriding-map-alist)))) ;; Define-minor-mode added our keymap to minor-mode-map-alist, but we want it ;; on minor-mode-overriding-map-alist instead. @@ -2953,7 +2972,32 @@ minibuffer-mode :interactive nil ;; Enable text conversion, but always make sure `RET' does ;; something. - (setq text-conversion-style 'action)) + (setq text-conversion-style 'action) + (when minibuffer-completion-visible + (setq-local minibuffer-completion-auto-choose nil))) + +(defcustom minibuffer-completion-visible t + "Non-nil means to navigate completions with arrows from the minibuffer. +This has effect only when the window with the *Completions* buffer +is visible on the screen." + :type 'boolean + :version "30.1") + +(defun minibuffer-bind-visible (binding) + `(menu-item + "" ,binding + :filter ,(lambda (cmd) + (when (get-buffer-window "*Completions*" 0) + cmd)))) + +(defvar-keymap minibuffer-local-visible-completion-map + :doc "Local keymap for minibuffer input with visible completions." + "<left>" (minibuffer-bind-visible #'minibuffer-previous-completion) + "<right>" (minibuffer-bind-visible #'minibuffer-next-completion) + "<up>" (minibuffer-bind-visible #'minibuffer-previous-line-completion) + "<down>" (minibuffer-bind-visible #'minibuffer-next-line-completion) + "RET" (minibuffer-bind-visible #'minibuffer-choose-completion) + "C-g" (minibuffer-bind-visible #'minibuffer-hide-completions)) ;;; Completion tables. @@ -4370,6 +4414,11 @@ completing-read-default ;; in minibuffer-local-filename-completion-map can ;; override bindings in base-keymap. base-keymap))) + (keymap (if minibuffer-completion-visible + (make-composed-keymap + (list minibuffer-local-visible-completion-map + keymap)) + keymap)) (buffer (current-buffer)) (c-i-c completion-ignore-case) (result @@ -4489,8 +4538,9 @@ minibuffer-completion-auto-choose :type 'boolean :version "29.1") -(defun minibuffer-next-completion (&optional n) +(defun minibuffer-next-completion (&optional n vertical) "Move to the next item in its completions window from the minibuffer. +When the optional argument VERTICAL is non-nil, move vertically. When `minibuffer-completion-auto-choose' is non-nil, then also insert the selected completion to the minibuffer." (interactive "p") @@ -4498,7 +4548,9 @@ minibuffer-next-completion (with-minibuffer-completions-window (when completions-highlight-face (setq-local cursor-face-highlight-nonselected-window t)) - (next-completion (or n 1)) + (if vertical + (next-line-completion (or n 1)) + (next-completion (or n 1))) (when auto-choose (let ((completion-use-base-affixes t)) (choose-completion nil t t)))))) @@ -4510,6 +4562,20 @@ minibuffer-previous-completion (interactive "p") (minibuffer-next-completion (- (or n 1)))) +(defun minibuffer-next-line-completion (&optional n) + "Move to the next completion line from the minibuffer. +When `minibuffer-completion-auto-choose' is non-nil, then also +insert the selected completion to the minibuffer." + (interactive "p") + (minibuffer-next-completion (or n 1) t)) + +(defun minibuffer-previous-line-completion (&optional n) + "Move to the previous completion line from the minibuffer. +When `minibuffer-completion-auto-choose' is non-nil, then also +insert the selected completion to the minibuffer." + (interactive "p") + (minibuffer-next-completion (- (or n 1)) t)) + (defun minibuffer-choose-completion (&optional no-exit no-quit) "Run `choose-completion' from the minibuffer in its completions window. With prefix argument NO-EXIT, insert the completion at point to the ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2023-11-01 17:45 ` Juri Linkov @ 2023-11-02 8:11 ` Eli Zaretskii 2023-11-02 17:14 ` Juri Linkov 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-11-02 8:11 UTC (permalink / raw) To: Juri Linkov; +Cc: 59486 > From: Juri Linkov <juri@linkov.net> > Date: Wed, 01 Nov 2023 19:45:50 +0200 > > Now here is a patch that implements the long-discussed feature of > using arrow keys for navigating completions from the minibuffer > only when the *Completions* buffer is visible. > > It's disabled by default and can be enabled by the option > 'minibuffer-completion-visible'. It works nicely for > the in-buffer completions as well: Thanks. I have a few comments. > +(defcustom minibuffer-completion-visible t > + "Non-nil means to navigate completions with arrows from the minibuffer. > +This has effect only when the window with the *Completions* buffer > +is visible on the screen." This doc string needs to be improved, as it currently doesn't explain its effect in enough detail, IMO. Maybe because "non-nil means to navigate" is awkward/confusing English. Regarding the last sentence of the doc string: does it mean that if the *Completions* buffer is not in any window, the arrow keys in the minibuffer will not move point in that buffer? If so, why this strange design? > +(defun minibuffer-bind-visible (binding) > + `(menu-item > + "" ,binding > + :filter ,(lambda (cmd) > + (when (get-buffer-window "*Completions*" 0) > + cmd)))) This function needs a doc string and possibly a better name, to match what it actually does. The argument zero to get-buffer-window AFAIU means that it will return non-nil when the buffer is shown in some window on an iconified frame, and I wonder why we would consider such a buffer "visible". > + "RET" (minibuffer-bind-visible #'minibuffer-choose-completion) AFAICT, this important binding is not mentioned or hinted in the doc string of the new option. > -(defun minibuffer-next-completion (&optional n) > +(defun minibuffer-next-completion (&optional n vertical) > "Move to the next item in its completions window from the minibuffer. > +When the optional argument VERTICAL is non-nil, move vertically. The "move vertically" part contradicts the "to the next item" part, doesn't it? Thus, I think the added sentence should be more detailed, and should explicitly say that the result will be a move to the next line, not the next item (and perhaps also include a link to next-line-completion). > +(defun minibuffer-next-line-completion (&optional n) > + "Move to the next completion line from the minibuffer. This sentence is misleading, IMO. Assuming I understood what you mean, I would rephrase: Move to the completion candidate on the next line in the completions buffer. > +When `minibuffer-completion-auto-choose' is non-nil, then also > +insert the selected completion to the minibuffer." Please make a habit of talking about "completion candidate" in these cases, not about "completion". The latter is ambiguous, since it can refer both to a candidate and to the action of performing completion. > +(defun minibuffer-previous-line-completion (&optional n) > + "Move to the previous completion line from the minibuffer. > +When `minibuffer-completion-auto-choose' is non-nil, then also > +insert the selected completion to the minibuffer." Same here. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2023-11-02 8:11 ` Eli Zaretskii @ 2023-11-02 17:14 ` Juri Linkov 2023-11-05 17:53 ` Juri Linkov 0 siblings, 1 reply; 15+ messages in thread From: Juri Linkov @ 2023-11-02 17:14 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 59486 [-- Attachment #1: Type: text/plain, Size: 3328 bytes --] >> +(defcustom minibuffer-completion-visible t >> + "Non-nil means to navigate completions with arrows from the minibuffer. >> +This has effect only when the window with the *Completions* buffer >> +is visible on the screen." > > This doc string needs to be improved, as it currently doesn't explain > its effect in enough detail, IMO. Maybe because "non-nil means to > navigate" is awkward/confusing English. Now improved in a new patch. > Regarding the last sentence of the doc string: does it mean that if > the *Completions* buffer is not in any window, the arrow keys in the > minibuffer will not move point in that buffer? If the *Completions* buffer is not in any window, the arrow keys move point in the minibuffer. Now explained this in the docstring. >> +(defun minibuffer-bind-visible (binding) >> + `(menu-item >> + "" ,binding >> + :filter ,(lambda (cmd) >> + (when (get-buffer-window "*Completions*" 0) >> + cmd)))) > > This function needs a doc string and possibly a better name, to match > what it actually does. Done. > The argument zero to get-buffer-window AFAIU means that it will return > non-nil when the buffer is shown in some window on an iconified frame, > and I wonder why we would consider such a buffer "visible". (get-buffer-window "*Completions*" 0) is used everywhere in minibuffer.el and simple.el, so the argument zero is for compatibility with other minibuffer completion commands. >> + "RET" (minibuffer-bind-visible #'minibuffer-choose-completion) > > AFAICT, this important binding is not mentioned or hinted in the doc > string of the new option. Now mentioned. >> -(defun minibuffer-next-completion (&optional n) >> +(defun minibuffer-next-completion (&optional n vertical) >> "Move to the next item in its completions window from the minibuffer. >> +When the optional argument VERTICAL is non-nil, move vertically. > > The "move vertically" part contradicts the "to the next item" part, > doesn't it? The next item can be on the axis x or y. > Thus, I think the added sentence should be more detailed, > and should explicitly say that the result will be a move to the next > line, not the next item (and perhaps also include a link to > next-line-completion). Ok, improved with a link. >> +(defun minibuffer-next-line-completion (&optional n) >> + "Move to the next completion line from the minibuffer. > > This sentence is misleading, IMO. Assuming I understood what you > mean, I would rephrase: > > Move to the completion candidate on the next line in the completions buffer. This line is too long and doesn't mention the minibuffer. So I added it to the rest of the docstring. >> +When `minibuffer-completion-auto-choose' is non-nil, then also >> +insert the selected completion to the minibuffer." > > Please make a habit of talking about "completion candidate" in these > cases, not about "completion". The latter is ambiguous, since it can > refer both to a candidate and to the action of performing completion. Ok, fixed. >> +(defun minibuffer-previous-line-completion (&optional n) >> + "Move to the previous completion line from the minibuffer. >> +When `minibuffer-completion-auto-choose' is non-nil, then also >> +insert the selected completion to the minibuffer." > > Same here. Here is a new patch: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: minibuffer-visible-completions.patch --] [-- Type: text/x-diff, Size: 6692 bytes --] diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 2120e31775e..56ba7e235f2 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -2707,8 +2716,14 @@ completion-in-region-mode completion-in-region-mode-predicate) (setq-local minibuffer-completion-auto-choose nil) (add-hook 'post-command-hook #'completion-in-region--postch) - (push `(completion-in-region-mode . ,completion-in-region-mode-map) - minor-mode-overriding-map-alist))) + (let* ((keymap completion-in-region-mode-map) + (keymap (if minibuffer-visible-completions + (make-composed-keymap + (list minibuffer-visible-completions-map + keymap)) + keymap))) + (push `(completion-in-region-mode . ,keymap) + minor-mode-overriding-map-alist)))) ;; Define-minor-mode added our keymap to minor-mode-map-alist, but we want it ;; on minor-mode-overriding-map-alist instead. @@ -2953,7 +2972,41 @@ minibuffer-mode :interactive nil ;; Enable text conversion, but always make sure `RET' does ;; something. - (setq text-conversion-style 'action)) + (setq text-conversion-style 'action) + (when minibuffer-visible-completions + (setq-local minibuffer-completion-auto-choose nil))) + +(defcustom minibuffer-visible-completions t + "When non-nil, visible completions can be navigated from the minibuffer. +This means that when the *Completions* buffer is visible in a window, +then you can use the arrow keys in the minibuffer to move the cursor +in the *Completions* buffer. Then you can type `RET', +and the candidate highlighted the *Completions* buffer +will be accepted. +But when the *Completions* buffer is not displayed on the screen, +then the arrow keys move point in the minibuffer as usual, and +`RET' accepts the input typed in the minibuffer." + :type 'boolean + :version "30.1") + +(defun minibuffer-visible-completions-bind (binding) + "Use BINDING when completions are visible. +Return an item that is enabled only when a window +displaying the *Completions* buffer exists." + `(menu-item + "" ,binding + :filter ,(lambda (cmd) + (when (get-buffer-window "*Completions*" 0) + cmd)))) + +(defvar-keymap minibuffer-visible-completions-map + :doc "Local keymap for minibuffer input with visible completions." + "<left>" (minibuffer-visible-completions-bind #'minibuffer-previous-completion) + "<right>" (minibuffer-visible-completions-bind #'minibuffer-next-completion) + "<up>" (minibuffer-visible-completions-bind #'minibuffer-previous-line-completion) + "<down>" (minibuffer-visible-completions-bind #'minibuffer-next-line-completion) + "RET" (minibuffer-visible-completions-bind #'minibuffer-choose-completion) + "C-g" (minibuffer-visible-completions-bind #'minibuffer-hide-completions)) ;;; Completion tables. @@ -4370,6 +4423,11 @@ completing-read-default ;; in minibuffer-local-filename-completion-map can ;; override bindings in base-keymap. base-keymap))) + (keymap (if minibuffer-visible-completions + (make-composed-keymap + (list minibuffer-visible-completions-map + keymap)) + keymap)) (buffer (current-buffer)) (c-i-c completion-ignore-case) (result @@ -4489,16 +4547,21 @@ minibuffer-completion-auto-choose :type 'boolean :version "29.1") -(defun minibuffer-next-completion (&optional n) +(defun minibuffer-next-completion (&optional n vertical) "Move to the next item in its completions window from the minibuffer. +When the optional argument VERTICAL is non-nil, move vertically +to the next item on the next line using `next-line-completion'. +Otherwise, move to the next item horizontally using `next-completion'. When `minibuffer-completion-auto-choose' is non-nil, then also -insert the selected completion to the minibuffer." +insert the selected completion candidate to the minibuffer." (interactive "p") (let ((auto-choose minibuffer-completion-auto-choose)) (with-minibuffer-completions-window (when completions-highlight-face (setq-local cursor-face-highlight-nonselected-window t)) - (next-completion (or n 1)) + (if vertical + (next-line-completion (or n 1)) + (next-completion (or n 1))) (when auto-choose (let ((completion-use-base-affixes t)) (choose-completion nil t t)))))) @@ -4506,17 +4569,35 @@ minibuffer-next-completion (defun minibuffer-previous-completion (&optional n) "Move to the previous item in its completions window from the minibuffer. When `minibuffer-completion-auto-choose' is non-nil, then also -insert the selected completion to the minibuffer." +insert the selected completion candidate to the minibuffer." (interactive "p") (minibuffer-next-completion (- (or n 1)))) +(defun minibuffer-next-line-completion (&optional n) + "Move to the next completion line from the minibuffer. +This means to move to the completion candidate on the next line +in the *Completions* buffer while point stays in the minibuffer. +When `minibuffer-completion-auto-choose' is non-nil, then also +insert the selected completion candidate to the minibuffer." + (interactive "p") + (minibuffer-next-completion (or n 1) t)) + +(defun minibuffer-previous-line-completion (&optional n) + "Move to the previous completion line from the minibuffer. +This means to move to the completion candidate on the previous line +in the *Completions* buffer while point stays in the minibuffer. +When `minibuffer-completion-auto-choose' is non-nil, then also +insert the selected completion candidate to the minibuffer." + (interactive "p") + (minibuffer-next-completion (- (or n 1)) t)) + (defun minibuffer-choose-completion (&optional no-exit no-quit) "Run `choose-completion' from the minibuffer in its completions window. -With prefix argument NO-EXIT, insert the completion at point to the -minibuffer, but don't exit the minibuffer. When the prefix argument +With prefix argument NO-EXIT, insert the completion candidate at point to +the minibuffer, but don't exit the minibuffer. When the prefix argument is not provided, then whether to exit the minibuffer depends on the value of `completion-no-auto-exit'. -If NO-QUIT is non-nil, insert the completion at point to the +If NO-QUIT is non-nil, insert the completion candidate at point to the minibuffer, but don't quit the completions window." (interactive "P") (with-minibuffer-completions-window ^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2023-11-02 17:14 ` Juri Linkov @ 2023-11-05 17:53 ` Juri Linkov 2023-11-15 17:45 ` Juri Linkov 0 siblings, 1 reply; 15+ messages in thread From: Juri Linkov @ 2023-11-05 17:53 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 59486 >> The argument zero to get-buffer-window AFAIU means that it will return >> non-nil when the buffer is shown in some window on an iconified frame, >> and I wonder why we would consider such a buffer "visible". > > (get-buffer-window "*Completions*" 0) is used everywhere in minibuffer.el > and simple.el, so the argument zero is for compatibility with other > minibuffer completion commands. During testing I discovered that the condition should be more complex. The problem is that the logic should be bound only to the minibuffer that showed the completions, not in other minibuffers within a set of recursive minibuffers. An example: `M-x TAB C-h v down RET' raised an error "Minibuffer is not active for completion". This error comes from `choose-completion-string', so I copied the same logic from `choose-completion-string': (when-let ((window (get-buffer-window "*Completions*" 0))) (when (eq (buffer-local-value 'completion-reference-buffer (window-buffer window)) (window-buffer (active-minibuffer-window))) cmd)) >> Same here. > > Here is a new patch: Now pushed the fixed patch. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#59486: completion-auto-wrap disobeyed by vertical navigation 2023-11-05 17:53 ` Juri Linkov @ 2023-11-15 17:45 ` Juri Linkov 0 siblings, 0 replies; 15+ messages in thread From: Juri Linkov @ 2023-11-15 17:45 UTC (permalink / raw) To: 59486 close 59486 30.0.50 thanks >> Here is a new patch: > > Now pushed the fixed patch. Looks like everything is done here, so closing. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-11-15 17:45 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-22 17:38 bug#59486: completion-auto-wrap disobeyed by vertical navigation Juri Linkov 2022-11-23 18:49 ` Juri Linkov 2022-11-24 7:59 ` Juri Linkov 2022-11-24 8:13 ` Eli Zaretskii 2022-11-24 18:30 ` Juri Linkov 2022-11-24 18:43 ` Eli Zaretskii 2022-11-25 7:47 ` Juri Linkov 2022-11-25 8:38 ` Eli Zaretskii 2022-11-28 7:56 ` Juri Linkov 2023-10-31 7:44 ` Juri Linkov 2023-11-01 17:45 ` Juri Linkov 2023-11-02 8:11 ` Eli Zaretskii 2023-11-02 17:14 ` Juri Linkov 2023-11-05 17:53 ` Juri Linkov 2023-11-15 17:45 ` Juri Linkov
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.