evil-cleverparens icon indicating copy to clipboard operation
evil-cleverparens copied to clipboard

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

Open bmag opened this issue 9 years ago • 7 comments

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,

bmag avatar Nov 12 '15 15:11 bmag

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.

luxbock avatar Nov 17 '15 05:11 luxbock

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

bmag avatar Nov 17 '15 06:11 bmag

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 avatar Nov 18 '15 17:11 justbur

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

justbur avatar Nov 18 '15 17:11 justbur

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
    1. 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)
    1. Open previously given file, make sure that it's in emacs-lisp-mode and activate evil-cleverparens-mode.
    1. 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.

bmag avatar Nov 25 '15 16:11 bmag

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!

luxbock avatar Nov 26 '15 14:11 luxbock

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))))
  1. 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.

Somelauw avatar Apr 15 '17 23:04 Somelauw