New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bmag opened this Issue Nov 12, 2015 · 7 comments

Comments

Projects
None yet
4 participants
@bmag

bmag commented Nov 12, 2015

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

This comment has been minimized.

Show comment
Hide comment
@luxbock

luxbock Nov 17, 2015

Owner

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.

Owner

luxbock commented Nov 17, 2015

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

This comment has been minimized.

Show comment
Hide comment
@bmag

bmag Nov 17, 2015

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

bmag commented Nov 17, 2015

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

@justbur

This comment has been minimized.

Show comment
Hide comment
@justbur

justbur Nov 18, 2015

Contributor

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

Contributor

justbur commented Nov 18, 2015

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

This comment has been minimized.

Show comment
Hide comment
@justbur

justbur Nov 18, 2015

Contributor

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

Contributor

justbur commented Nov 18, 2015

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

@bmag

This comment has been minimized.

Show comment
Hide comment
@bmag

bmag Nov 25, 2015

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 commented Nov 25, 2015

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.
@luxbock

This comment has been minimized.

Show comment
Hide comment
@luxbock

luxbock Nov 26, 2015

Owner

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!

Owner

luxbock commented Nov 26, 2015

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 pushed a commit that referenced this issue Nov 27, 2015

orlandow added a commit to orlandow/evil-cleverparens that referenced this issue Aug 12, 2016

orlandow added a commit to orlandow/evil-cleverparens that referenced this issue Aug 12, 2016

@Somelauw

This comment has been minimized.

Show comment
Hide comment
@Somelauw

Somelauw Apr 15, 2017

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 commented Apr 15, 2017

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.

luxbock added a commit that referenced this issue Jul 18, 2017

yuhan0 added a commit to yuhan0/evil-cleverparens that referenced this issue Sep 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment