Locked cycling in yank-pop

[alternateved]

Hello Prot,

It seems that I've found a small thing that might be worth investigating.

If the kill-ring gathers enough whitespace in some positions, the cycling between completion candidates gets locked. This happens not only with whitespace, but whitespace is the easiest to reproduce. Using C-n to go to the next completion candidate is impossible, there is a need to either use C-p and cycle trough everything from the other side or just use the mouse.

This issue appears in built-in yank-pop and also in consult-yank-pop. From what I've investigated, this doesn't happen in Vertico, Icomplete or Fido.

Below is a screenshot which presents minimal configuration (although with straight.el) where the cycling cannot go past third candidate

Best wishes, alternateved

[protesilaos]

Hello @alternateved and thank you for reporting this bug!

It seems that I've found a small thing that might be worth investigating.

Good catch! This has to do with the heuristic for finding and skipping over the last empty line in the *Completions* buffer. This change seems to fix the issue, but it looks a bit ugly.

diff --git a/mct.el b/mct.el
index 1784504..443ce50 100644
--- a/mct.el
+++ b/mct.el
@@ -604,12 +636,16 @@ (defun mct--bottom-of-completions-p (arg)
       (mct--completions-line-boundary (mct--last-completion-point))
       (= (save-excursion (next-completion arg) (point)) (point-max))
       ;; The empty final line case...
-      (save-excursion
-        (goto-char (point-at-bol))
-        (and (not (bobp))
-            (or (beginning-of-line (1+ arg)) t)
-            (save-match-data
-              (looking-at "[\s\t]*$"))))))
+      (eq (save-excursion
+            (mct--last-completion-point)
+            (vertical-motion 1)
+            (point))
+          (save-excursion
+            (goto-char (point-at-bol))
+            (and (not (bobp))
+                (or (beginning-of-line (1+ arg)) t)
+                (save-match-data
+                  (looking-at "[\s\t]*$")))))))

 (defun mct--next-completion (arg)
   "Routine to move to the next ARGth completion candidate."

I will review it tomorrow morning and test it a bit further before pushing a fix.

EDIT: remove typo.

[alternateved]

That is great! I seriously need to get better with lisp in order to bring a little bit more to the table. But I am glad that it wasn't hard to locate the source of the problem.

[protesilaos]

What I shared last night was not good enough. I have learnt not to make tricky changes before going to bed. This morning I reviewed things and committed this instead:

 mct.el | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/mct.el b/mct.el
index 586cf44..6e1698a 100644
--- a/mct.el
+++ b/mct.el
@@ -524,6 +524,12 @@ (defun mct--completions-completion-p ()
     (or (get-text-property point 'completion--string)
         (get-text-property point 'mouse-face))))

+(defun mct--arg-completion-point-p (arg)
+  "Return non-nil if ARGth next completion exists."
+  (save-excursion
+    (mct--next-completion arg)
+    (mct--completions-completion-p)))
+
 (defun mct--first-completion-point ()
   "Return the `point' of the first completion."
   (save-excursion
@@ -596,6 +602,16 @@ (defun mct-switch-to-completions-bottom ()
            (truncate (/ (window-body-height) 4.0))))
    t))

+(defun mct--empty-line-p (arg)
+  "Return non-nil if ARGth line is empty."
+  (unless (mct--arg-completion-point-p arg)
+    (save-excursion
+      (goto-char (point-at-bol))
+      (and (not (bobp))
+          (or (beginning-of-line (1+ arg)) t)
+          (save-match-data
+            (looking-at "[\s\t]*$"))))))
+
 (defun mct--bottom-of-completions-p (arg)
   "Test if point is at the notional bottom of the Completions.
 ARG is a numeric argument for `next-completion', as described in
@@ -603,13 +619,9 @@ (defun mct--bottom-of-completions-p (arg)
   (or (eobp)
       (mct--completions-line-boundary (mct--last-completion-point))
       (= (save-excursion (next-completion arg) (point)) (point-max))
-      ;; The empty final line case...
-      (save-excursion
-        (goto-char (point-at-bol))
-        (and (not (bobp))
-            (or (beginning-of-line (1+ arg)) t)
-            (save-match-data
-              (looking-at "[\s\t]*$"))))))
+      ;; The empty final line case which should avoid candidates with
+      ;; spaces or line breaks...
+      (mct--empty-line-p arg))))

 (defun mct--next-completion (arg)
   "Routine to move to the next ARGth completion candidate."

Please test it. If all is good, we can close this issue.

---

seriously need to get better with lisp in order to bring a little bit more to the table.

I think learning Elisp is useful for every Emacs user: it helps you get more out of Emacs. I started with no knowledge of Lisp and am learning new tricks every day, which lets me customise my workflow exactly how I want (beside the inherent value of learning and having fun with code).

For our purposes though, it is not necessary to contribute code. I believe that a bug report is already a major contribution. This is why in all my manuals I have a section titled "Acknowledgements" where I include the names of people who have contributed to the project in one way or another.

But I am glad that it wasn't hard to locate the source of the problem.

It wasn't difficult because I am familiar with the code. Otherwise it would have taken much longer.

[alternateved]

I think learning Elisp is useful for every Emacs user: it helps you get more out of Emacs. I started with no knowledge of Lisp and am learning new tricks every day, which lets me customise my workflow exactly how I want (beside the inherent value of learning and having fun with code).

I wholeheartedly agree with that statement. I will get there, eventually. There is always so much to learn, especially when it comes to programming.

For our purposes though, it is not necessary to contribute code. I believe that a bug report is already a major contribution. This is why in all my manuals I have a section titled "Acknowledgements" where I include the names of people who have contributed to the project in one way or another.

I see it now, thank you! That is really kind of you.

I've tested the changes and everything seems to be working now as it should. I've also tested the changes from previous issue (the one in mct-highlight-candidate face) and now it looks more natural when using mct with other themes.

I think that might be it for that issue, thank you for your help.

I might have something more to report, but I will move it to GitLab next time as I think this is where the most traffic is happening.

[protesilaos]

On 2022-01-28, 00:16 -0800, Tomasz Hołubowicz @.***> wrote:

I think learning Elisp is useful for every Emacs user: it helps you get more out of Emacs. I started with no knowledge of Lisp and am learning new tricks every day, which lets me customise my workflow exactly how I want (beside the inherent value of learning and having fun with code).

I wholeheartedly agree with that statement. I will get there, eventually. There is always so much to learn, especially when it comes to programming.

Very well! Indeed, there is always something new to learn. I think that is part of the fun.

For our purposes though, it is not necessary to contribute code. I believe that a bug report is already a major contribution. This is why in all my manuals I have a section titled "Acknowledgements" where I include the names of people who have contributed to the project in one way or another.

I see it now, thank you! That is really kind of you.

When the modus-themes were merged into emacs.git I did a video where I explained how I see it as a community effort and thanked everyone for their contributions up until that time (bug reports, ideas, code patches, ...). I think it is important to acknowledge this aspect of the development process even if most of the coding/design comes from one person.

I've tested the changes and everything seems to be working now as it should. I've also tested the changes from previous issue (the one in mct-highlight-candidate face) and now it looks more natural when using mct with other themes.

Great!

I think that might be it for that issue, thank you for your help.

You are welcome!

I might have something more to report, but I will move it to GitLab next time as I think this is where the most traffic is happening.

Whatever works for you. I maintain both repos to make it easier for people.

-- Protesilaos Stavrou https://protesilaos.com