Slightly wrong behavior for evil-cp-forward-sexp and evil-cp-backward-sexp at end of line

[bmag]

Hey, first of all thanks for this package, I think it's great.

I noticed a slightly annoying behavior for evil-cp-forward-sexp and evil-cp-backward-sexp (L and H). Consider this elisp file (| denotes the cursor):

'(|("aaa" . bbb)
  ("ccc" . ddd))

If evil-move-beyond-eol is t, then pressing L once moves the cursor after "bbb)":

'(("aaa" . bbb)|
  ("ccc" . ddd))

From here, I can press either L or H. If I press L:

'(("aaa" . bbb)
  ("ccc" . ddd)|)

Or if I press H:

'(|("aaa" . bbb)
  ("ccc" . ddd))

Which is exactly what I expect to happen, so I consider it a good behavior.

On the other hand, if evil-move-beyond-eol is nil, the behavior is a bit off. Starting from the same place as before:

'(|("aaa" . bbb)
  ("ccc" . ddd))

Pressing L once moves the cursor after "bbb" but before the ")". Because evil-move-beyond-eol is nil, this is the expected behavior (so far so good).

'(("aaa" . bbb|)
  ("ccc" . ddd))

From here, however, pressing either L or H doesn't bring me to where I want. If I press L, the cursor doesn't move at all.
If I press H, the cursor moves in front of "bbb" instead of returning to where it was:

'(("aaa" . |bbb)
  ("ccc" . ddd))

The two problems here are that in this case H doesn't do the reverse of L, and that I can't move forward in the list because of how evil-mode adjusts the cursor.
Will it be possible to modify evil-cp-forward-sexp and evil-cp-backward-sexp for the case where the cursor is on a closing paren and evil-move-beyond-eol is nil? If not,

[luxbock]

I can't reproduce this. I use evil-move-beyond-eol with nil and I get the behavior you describe at first. I don't actually fully understand what evil-move-beyond-eol does so I might be missing something.

[bmag]

Really? I'll try later this week to reproduce it with a minimal config

[justbur]

I believe evil-move-beyond-eol (when nil) simply moves the point back one char if you move to the eol, so @bmag's behavior makes sense

[justbur]

Oh, that's true but it also depends on the value of evil-move-cursor-back

[bmag]

Ok, I've got a reproduction recipe. I've noticed that if evil-move-cursor-back is nil, or evil-move-beyond-eol is t, then the behavior is ok. But if evil-move-cursor-back is t and evil-move-beyond-eol is nil, and these are also the default values, then I get the wrong behavior.

The recipe:

• 1. Start a config-less emacs: emacs -Q
• 2. Evaluate in scratch buffer:

(add-to-list 'load-path "/path/to/evil/")
(add-to-list 'load-path "/path/to/goto-chg/")
(add-to-list 'load-path "/path/to/undo-tree/")
(require 'evil)
(evil-mode +1)

(add-to-list 'load-path "/path/to/evil-cleverparens/")
(add-to-list 'load-path "/path/to/dash/")
(add-to-list 'load-path "/path/to/smartparens/")
(add-to-list 'load-path "/path/to/paredit/")
(require 'evil-cleverparens)

• 3. Open previously given file, make sure that it's in emacs-lisp-mode and activate evil-cleverparens-mode.
• 4. For different combinations of evil-move-beyond-eol and evil-move-cursor-back values (M-: is your friend), check which of the behaviors in the original post you experience.
     When evil-move-cursor-back is t and evil-move-beyond-eol is nil, you should get the wrong behavior. For any of the other 3 possible combinations, you should get the good behavior.

[luxbock]

Thanks for the reproduction steps, I can see what the problem is now. With evil-move-cursor-back set to t and evil-move-beyond-eol set to nil there is simply no more space for the point to move to when it reaches the closing parentheses in your example, which then breaks the symmetry of movement between H and L. Note that the command would work fine if there was an extra space at the end of the first list we're jumping over.

I've personally always used evil with evil-move-cursor-back set to nil so this problem never occured to me, and I only just now realized the purpose of evil-move-beyond-eol. I can't really come up with any other solutions to this problem (open to suggestions) besides recommending you to turn this setting on. Since the combination of these two settings is the default it should probably be mentioned in the README!

[Somelauw]

Here are some ideas:

1. Modify evil-forward-sexp to move point to some place at the beginning of the next line if the underlying function sp-forward-sexp ends at eol.

Something like:

  (evil-define-motion evil-cp-end-of-defun (count)
    "Motion for moving forward by a sexp via `sp-forward-sexp'."
    :type exclusive
    (let ((count (or count 1)))
      (sp-forward-sexp count)
      (when (and evil-move-cursor-back (not evil-move-beyond-eol) (eolp))
        (forward-char 1))))

2. Bind L to evil-cp-next-sexp instead. This function hasn't been written yet, but is easy to implement since sp-next-sexp already exists.

[bo-tato]

I patched this in my config with:

(defun my/evil-eol-advice (&optional count)
  (when (evil-eolp)
    (forward-char)))

(advice-add 'sp-forward-sexp :before #'my/evil-eol-advice)

There is an open issue on smartparens Fuco1/smartparens#1037 which is where it should probably be fixed but I'm not sure how much they care about fixing evil mode edge cases. I think evil-cleverparens could fix it by just adding:

(when (evil-eolp)
    (forward-char))

before calling sp-forward-sexp in evil-cp-forward-sexp

[bo-tato]

It seems that first hacky advice-add before sp-forward-sexp broke the behaviour of evil-cp-delete-char-or-splice when the closing paren is at end of line, and probably other things that depend on sp-forward-sexp. I'm now trying the second suggestion of adding eol check to evil-cp-forward-sexp and haven't noticed anything broken yet

[tomdl89]

Nice! I'll take a look soon and can make the change if it looks good

[tomdl89]

Added the fix suggested by @bo-tato to master, so evil-cp-forward-sexp should now work at eol, even if evil-move-beyond-eol is nil. Lemme know if you observe otherwise.