emacsfodder / move-text

move current line or region up or down
199 stars 12 forks source link

Move region doesn't work correctly in evil-mode #11

Closed linchen2chris closed 2 years ago

linchen2chris commented 7 years ago

just as the title

jasonm23 commented 7 years ago

Hi @linchen2chris I don't explicitly support eVil mode so if this is something you'd​ like to see done quickly, please submit a patch / pull-request

edmundmiller commented 6 years ago

I plan on getting this working. @jasonm23 could you explain this function? move-text-get-region-and-prefix

jasonm23 commented 6 years ago

It replaces "r" in (interactive) (which is the usual way to get the current region.)

Let's look at the business end of that macro.

`(,@(if (region-active-p)
            (list (region-beginning) (region-end))
          (list nil nil))
      ,current-prefix-arg))

This macro expansion will spew out the values of region-beginning and region-end as a list if there's an active region (otherwise it's a list of '(nil nil)), it also passes on current-prefix-arg (in either case)

Interactive will just pass through a list of arguments and they become the defun args...

Let's look at a use case in the rest of move-text.el

You can see here we replaced (interactive "r\np") with a use of move-text-get-region-and-prefix

(defun move-text-region (start end n)
   "Move the current region (START END) up or down by N lines."
-  (interactive "r\np")
+  (interactive (move-text-get-region-and-prefix))

So the list that was built by (move-text-get-region-and-prefix) is what (interactive) will use to fill in the defun parameters, start, end, n (region and argument prefix)

Superfluous notes...

NOTE: This is all much easier to understand if we call that macro (+get-region-and-prefix)

(It's a recommended Emacs Lisp practice to use the package name as a namespacing prefix gimmick, to avoid the troubles of global scope and cooking up your own way of dealing with that problem. In Lisp we are screwed by lack of conventions, so it's useful to stick to the ones in the prevailing lisp-subculture)

jasonm23 commented 6 years ago

I wanted to show you how that macro looks to me in Emacs...

image

jasonm23 commented 6 years ago

Note I call this thing a macro because it is used with (interactive) and it adds itself into that macro.

Nebucatnetzer commented 5 years ago

just as the title

To me this issue is not clear. Move-text seems to work just fine on my system with evil. What doesn't work is obviously doesn't work is moving the text with M-j and M-k however that's not really something one would expect to work after reading the readme file.

Since org-mode uses the same keybindings it feels actually quite natural to me.

oyren commented 5 years ago

just as the title

To me this issue is not clear. Move-text seems to work just fine on my system with evil. What doesn't work is obviously doesn't work is moving the text with M-j and M-k however that's not really something one would expect to work after reading the readme file.

Since org-mode uses the same keybindings it feels actually quite natural to me.

+1 is the same for me

rnoennig commented 3 years ago

When I change move-text's move-text-region like this (added the when):

(defun move-text-region (start end n)
  "Move the current region (START END) up or down by N lines."
  (interactive (move-text-get-region-and-prefix))
  (let ((line-text (delete-and-extract-region start end)))
    (forward-line n)
    (let ((start (point)))
      (insert line-text)
      (set-mark start)
      (setq deactivate-mark nil)
      (when (and (bound-and-true-p evil-mode) (string= evil-state "visual"))
        (backward-char 1)
        (evil-visual-make-selection (mark) (point)))
      )))

then moving text around in evil's VISUAL LINE moves the lines around like expected. gif animation of move-text-up and move-text-down with modded move-text-region

Can't say the same for VISUAL or VISUAL BLOCK and am actually quite unsure if there is a common agreement on how the movement should actually work.

What requirements do you have to accept a pull request? @jasonm23 I'd be happy to support working on this.

jasonm23 commented 3 years ago

Hi there's no intention in Move-text to support vi mode.

However a fully tested solution which leaves Emacs editing modes unaffected would be considered.

My recommendation however is to fork an eVil specific version to simplify and avoid breaking changes to Move-text.

May I suggest calling it Evil-move-text?

(I'm a bit surprised that this isn't already a thing!)

rnoennig commented 3 years ago

What a bummer. Before starting a fork I'd recommend replacing move-text with drag-stuff. Anyway thanks for the reply.

Profpatsch commented 2 years ago

I can confirm that drag-stuff works just fine with selections in evil-mode.

8dcc commented 2 months ago

Before starting a fork I'd recommend replacing move-text with drag-stuff.

That package has compilation errors, hasn't been updated in ages, and works terribly.

I can confirm that drag-stuff works just fine with selections in evil-mode.

This is not the case for me, perhaps because I use transient-mark-mode.