From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh Newsgroups: gmane.emacs.bugs Subject: bug#68688: 30.0.50; next-line-completion doesn't work with multiline completions Date: Wed, 24 Jan 2024 13:27:39 -0500 Message-ID: References: <86ttn23ckr.fsf@mail.linkov.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="12689"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 68688@debbugs.gnu.org To: Juri Linkov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Jan 24 19:28:19 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1rShz0-00036u-SJ for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 24 Jan 2024 19:28:19 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rShyg-0005QO-8J; Wed, 24 Jan 2024 13:27:58 -0500 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 1rShyf-0005QE-14 for bug-gnu-emacs@gnu.org; Wed, 24 Jan 2024 13:27:57 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rShye-0008UF-O9 for bug-gnu-emacs@gnu.org; Wed, 24 Jan 2024 13:27:56 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rShyj-0007te-NG for bug-gnu-emacs@gnu.org; Wed, 24 Jan 2024 13:28:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 24 Jan 2024 18:28:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 68688 X-GNU-PR-Package: emacs Original-Received: via spool by 68688-submit@debbugs.gnu.org id=B68688.170612087230338 (code B ref 68688); Wed, 24 Jan 2024 18:28:01 +0000 Original-Received: (at 68688) by debbugs.gnu.org; 24 Jan 2024 18:27:52 +0000 Original-Received: from localhost ([127.0.0.1]:46557 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rShya-0007tE-48 for submit@debbugs.gnu.org; Wed, 24 Jan 2024 13:27:52 -0500 Original-Received: from mxout5.mail.janestreet.com ([64.215.233.18]:53901) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rShyY-0007sz-Iu for 68688@debbugs.gnu.org; Wed, 24 Jan 2024 13:27:51 -0500 In-Reply-To: <86ttn23ckr.fsf@mail.linkov.net> (Juri Linkov's message of "Wed, 24 Jan 2024 18:45:08 +0200") DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1706120859; bh=YsUIHvlZG1j7ws2lXU7A2O1SLrULjCPuogMZ7inoYUg=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=TTtMGGoFEPoMk2IsnjaOng/WlyBo3YxCjojZFue2ABg07PcfxgMC0mb+Pa1r30Rf6 c8B6Mhy3uit1pB5rhe5x1cDjWkOv/9cCJfAQS+MN6PokFRaeZQJb1SlNrx/zncgxie 4Zc3CciNn+0jATpfsNoRqtWlWjOJNlvKwH3u812b+ePTHmCKTUAcr/5QYAVI51ulZB wQEN7czoiGtOB6eK63PcSisV8l9O5559dFMZj3eIooY+OFB1EARgiigjztKS+39miY vY76/V2R95ND6rxcgFBASQGQtBGsnt/2XWgxaejSbM5/yZFR+LTmxq0dZMGrC0Lqa4 EDTNcxi0oT1Zg== X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:278811 Archived-At: --=-=-= Content-Type: text/plain Juri Linkov writes: >> Patch which fixes this attached. > > Thanks for fixing the multi-line case. Could you please > add a test in minibuffer-tests.el. Maybe easier is > to replace '("aa" "ab" "ac") with '("aa" "a\nb" "ac"). > Or a new test would be better, I don't know. Sure. New test is easier since "a\nb" sorts before "aa", so adding the newline changes the test results. >> It simply adds an additional loop around forward-line which runs >> until we leave the completion at point. > > Alternatively without a loop maybe simpler would be just to move > to the end of the current completion before calling forward-line? I was unsure whether that would work correctly in all cases. But it seems to pass tests, so let's just try it. If it does break, the bug report will give us some new test cases :) >> BTW, I think we should have some helper functions for "get start of >> completion candidate or point-min" and "get end of completion candidate >> or point-max", or possibly "move to start of candidate" and "move to end >> of candidate"; it's pretty hard to get right. > > Completely agreed. We need more helper functions for all 4 functions: > previous-completion, next-completion, previous-line-completion, > next-line-completion. Cool, I made two helpers in the latest version of my patch. Using only in next-line-completion for now, but we can use it more in the other completion function. > BTW, do you think that these parts below are not needed anymore > since you implemented a hidden placeholder at the top > in case of no header? Yes, I think all the code for 'first-completion can be removed. Updated patch for next-line-completion: --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Fix-next-line-completion-for-multi-line-completions.patch >From cbc8bdc1386a4bc9c420d8ab06b844c6f6928c35 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Wed, 24 Jan 2024 10:52:40 -0500 Subject: [PATCH] Fix next-line-completion for multi-line completions Previously it would not move out of a multi-line completion, and now it will. * lisp/simple.el (next-line-completion): Move to the completion start or end before going forward or backward lines. (bug#68688) --- lisp/simple.el | 20 +++++++++++++++++--- test/lisp/minibuffer-tests.el | 14 ++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lisp/simple.el b/lisp/simple.el index 692c0dacefc..4ffe159dc88 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -9940,6 +9940,20 @@ previous-completion (interactive "p") (next-completion (- n))) +(defun completion--move-to-candidate-start () + "If in a completion candidate, move point to its start." + (when (and (get-text-property (point) 'mouse-face) + (not (bobp)) + (get-text-property (1- (point)) 'mouse-face)) + (goto-char (previous-single-property-change (point) 'mouse-face)))) + +(defun completion--move-to-candidate-end () + "If in a completion candidate, move point to its end." + (when (and (get-text-property (point) 'mouse-face) + (not (eobp)) + (get-text-property (1+ (point)) 'mouse-face)) + (goto-char (or (next-single-property-change (point) 'mouse-face) (point-max))))) + (defun next-completion (n) "Move to the next item in the completions buffer. With prefix argument N, move N items (negative N means move @@ -10029,9 +10043,7 @@ next-line-completion (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))) + (completion--move-to-candidate-start) ;; Try to move to the previous completion. (setq pos (previous-single-property-change (point) 'mouse-face)) (if pos @@ -10046,6 +10058,7 @@ next-line-completion (while (> n 0) (setq found nil pos nil column (current-column) line (line-number-at-pos)) + (completion--move-to-candidate-end) (while (and (not found) (eq (forward-line 1) 0) (not (eobp)) @@ -10070,6 +10083,7 @@ next-line-completion (while (< n 0) (setq found nil pos nil column (current-column) line (line-number-at-pos)) + (completion--move-to-candidate-start) (while (and (not found) (eq (forward-line -1) 0) (eq (move-to-column column) column)) diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index c1fe3032cb5..d104858b0d0 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -465,6 +465,20 @@ completion-auto-wrap-test (previous-line-completion 4) (should (equal "ac" (get-text-property (point) 'completion--string)))))) +(ert-deftest completion-next-line-multline-test () + (let ((completion-auto-wrap t)) + (completing-read-with-minibuffer-setup + '("a\na" "a\nb" "ac") + (insert "a") + (minibuffer-completion-help) + (switch-to-completions) + (goto-char (point-min)) + (next-line-completion 5) + (should (equal "a\nb" (get-text-property (point) 'completion--string))) + (goto-char (point-min)) + (previous-line-completion 5) + (should (equal "a\nb" (get-text-property (point) 'completion--string)))))) + (ert-deftest completions-header-format-test () (let ((completion-show-help nil) (completions-header-format nil)) -- 2.39.3 --=-=-=--