Locked cycling in yank-pop
alternateved opened this issue · 5 comments
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
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.
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.
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.
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.