evil-scroll-down defined as next-line command

[fast-90]

Issue type

• Question

Environment

Emacs version: GNU Emacs 29.0.50 (build 1, x86_64-apple-darwin21.5.0, NS appkit-2113.50 Version 12.4 (Build 21F79)) of 2022-07-10
Operating System: MacOS Montery version 12.5
Evil version: Evil version 1.15.0
Evil installation type: straight (via MELPA I believe)
Graphical/Terminal: Graphical
Tested in a make emacs session (see CONTRIBUTING.md): No

Reproduction steps

• Start Emacs
• Install pulsar
• Add the following config:

(require 'pulsar)
(pulsar-global-mode 1)
(add-to-list 'pulsar-pulse-functions 'evil-scroll-down)

• Press C-d to scroll down

Expected behavior

When executing evil-scroll-down (e.g. through C-d), the line the cursor is on should pulse (temporarily highlight as intended by pulsar).

Actual behavior

Nothing happens.

Further notes

The issue is likely caused due to evil defining evil-scroll-down as a next-line command, see here (special thanks to @protesilaos for identifying the root cause of the issue). The same issue happens with evil-scroll-up.

Adding next-line to pulsar-pulse-functions makes it work, but has the undesired effect of making the line alsopulse for the j motion.

Added this a question instead of a bug-report as it might be an edge case where defining it as next-line is actually an issue. My question is: what is the reason it is defined like this (and not scroll-down-command for example)? And are there other ways around my problem is the scroll down function has to be defined as a next-line command?

[tomdl89]

Usually we spoof a command as next-line because we want it to preserve the column of the cursor while passing over lines that may be narrower than said column. Emacs facilitates such column preservation by checking if the previous command was next-line or previous-line which isn't ideal, because it means evil has to pretend a command is one of these if it wants to leverage this functionality. Otherwise we'd have to manage the column preservation ourselves, which there has been no appetite to do. One thing you could do is add your own post-command-hook which checks if real-this-command is evil-scroll-down and pulses as required. Seems a bit hacky for the lib itself, but in an init.el it's probably tolerable.

[fast-90]

@tomdl89 Thanks for the clarification and the tip to solve the issue (I was unaware of the real-this-command variable)! I have successfully added the post-command-hook in my config, and it works for me now.

@protesilaos I will leave it up to you to decide whether or not you want to make any adjustments to pulsar (although might be worth adding some documentation on this issue anyways as I think there may be quite some evil users out there also trying to use pulsar).

In case people are facing the same issue and stumble on this thread trying to find a solution, the following has worked for me:

(defun my/pulsar-pulse-evil-scroll ()
  (when (or (eq real-this-command 'evil-scroll-down)
            (eq real-this-command 'evil-scroll-up))
    (pulsar-pulse-line)))

(add-hook 'post-command-hook #'my/pulsar-pulse-evil-scroll)

Not sure if that is the best way to do it as I'm not a lisp expert by any means :)

[protesilaos]

Thank you @tomdl89!

@fast-90 I think we can continue this discussion on the mailing list. I believe the following should work:

 pulsar.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pulsar.el b/pulsar.el
index ed62b86..0e2506f 100644
--- a/pulsar.el
+++ b/pulsar.el
@@ -393,7 +393,8 @@ (define-globalized-minor-mode pulsar-global-mode pulsar-mode pulsar--on)
 (defun pulsar--post-command-pulse ()
   "Run `pulsar-pulse-line' for `pulsar-pulse-functions'."
   (when (and (or pulsar-mode pulsar-global-mode)
-             (memq this-command pulsar-pulse-functions))
+             (or (memq this-command pulsar-pulse-functions)
+                 (memq real-this-command pulsar-pulse-functions)))
     (pulsar-pulse-line)))
 
 (make-obsolete 'pulsar-setup nil "0.3.0")

[fast-90]

Closing this as explanation sufficiently provided by @tomdl89 and original issue resolved by @protesilaos in the new update of pulsar.