move-text-down does not maintain proper selection when using visual line mode

aaronjensen commented 8 years ago

Description

If you select multiple lines with visual line mode V and then M-x move-text-down the resulting selection is incorrect. You should be able to repeat M-x move-text-down and continue to move the same text without having to reselect. Compare to hybrid/holy mode when you use C-SPC C-n C-n to select multiple lines.

Note, move-text-down and move-text-up are bound to ] e and [ e respectively by https://github.com/syl20bnr/spacemacs/blob/074f425dc5d233f24195ecc3021eb96ac9d55d4d/layers/%2Bvim/unimpaired/keybindings.el

Also, SPC x J is bound as part of core and it is broken as well in visual line mode.

Reproduction guide

Start Emacs in evil mode
SPC b s

Enter text:

line 1
line 2
line 3
line 4
line 5
line 6

Go to line 3 and V j j
M-x move-text-down (can also use SPC x J)
M-x move-text-down

Observed behaviour:

Text is moved correctly, but line 5 is selected after first move-text-down. After second, line 5 moves down a line. Resulting in 1, 2, 3, 5, 4, 6.

visual

Expected behaviour:

After first move-text-down lines 2 and 3 should remain selected. The second should move them down again, making the line order 1, 2, 5, 6, 3, 4. This is how it works if you were to use hybrid/holy mode and select with C-SPC C-n C-n instead.

emacs

System Info

OS: darwin
Emacs: 25.1.50.1
Spacemacs: 0.105.11
Spacemacs branch: enable-non-native-fullscreen-without-mac-port (rev. 891e704)
Graphic display: t
Distribution: spacemacs
Editing style: hybrid
Completion: helm
Layers:

(better-defaults spacemacs-layouts spacemacs-helm emacs-lisp markdown
                 (syntax-checking :variables syntax-checking-enable-tooltips nil)
                 (auto-completion :variables auto-completion-enable-sort-by-usage t)
                 erlang elixir git dash html org colors osx github javascript deft floobits ruby semantic
                 (shell :variables shell-default-shell 'ansi-term shell-default-height 30 shell-default-position 'bottom)
                 spell-checking ranger version-control rcirc evil-little-word jb-lispy auto-correct frame-geometry)

aaronjensen commented 8 years ago

Reported in evil as well: https://bitbucket.org/lyro/evil/issues/630

aaronjensen commented 8 years ago

FYI, closed as invalid on evil. We may need our own version of move-text-down that works in evil.

StreakyCobra commented 8 years ago

Could not this be reported upstream on move-text maybe? Just askin'…

jcalve commented 8 years ago

move-text doesn't have an upstream repo as far as I can tell, emacswiki links to their own repo.

I've been searching for a replacement for this too for a couple of days and I haven't found any that works well with evil, which is rather anoying since this is a one liner for each direction in vim:

vnoremap J :m '>+1<CR>gv=gv
vnoremap K :m '<-2<CR>gv=gv

aaronjensen commented 8 years ago

@jcalve I've got it close, but it doesn't quite work, see http://emacs.stackexchange.com/questions/20782/programmatically-execute-evil-ex-move-command

aaronjensen commented 8 years ago

@jcalve w/ the latest, soon to be on MELPA evil, this works:

(define-key evil-visual-state-map "J"
  (concat ":m '>+1" (kbd "RET") "gv=gv"))
(define-key evil-visual-state-map "K"
  (concat ":m '<-2" (kbd "RET") "gv=gv"))

jcalve commented 8 years ago

@aaronjensen that's great, mr. Fischer delivering the goods, I think that closes this issue

aaronjensen commented 8 years ago

I'm going to close this out, but I wonder if it'd make sense to update https://github.com/syl20bnr/spacemacs/blob/074f425dc5d233f24195ecc3021eb96ac9d55d4d/layers/%2Bvim/unimpaired/keybindings.el#L9-L10 to at least do gv afterwards?

syl20bnr commented 8 years ago

I re-open to fix the key bindings in related evil states.

aaronjensen commented 8 years ago

@syl20bnr are you just thinking of just making the visual bindings as I have J/K up there? If so, can pull request, if not, just let me know what you're thinking

onemanstartup commented 8 years ago

Is it possible to use Ctrl+j ? In vim J even in visual mode is concatenation of lines.

YayC commented 8 years ago

How about move-text-up in normal mode? If the cursor is not at column 0, it adds spaces on the line above: output

YayC commented 8 years ago

^on latest master (just updated packages)

System Info

OS: darwin
Emacs: 25.1.1
Spacemacs: 0.105.22
Spacemacs branch: master (rev. 9f9faa4)
Graphic display: t
Distribution: spacemacs
Editing style: vim
Completion: helm
Layers:

(auto-completion emacs-lisp sql elixir
                 (ruby :variables ruby-version-manager 'rvm ruby-enable-enh-ruby-mode t)
                 ruby-on-rails dash markdown org vinegar)

dieggsy commented 7 years ago

@aaronjensen I don't personally use spacemacs, but I found this conversation interesting. This is more of a thought aloud, and I don't know if this will actually be useful to anyone, but perhaps this could be more generalized into moving both lines and regions based on context? Kind of like this (excuse my probably messy emacs-lisp):

(defun move-line-or-region (arg)
  (interactive "P")
  (if (or (not arg) (>= arg 0))
      (let ((reg-or-lin (if (region-active-p) "'>" "."))
            (reactivate-region (if (region-active-p) "gv=gv" ""))
            (num (if arg arg 1)))
        (execute-kbd-macro
         (concat ":m" reg-or-lin "+" (number-to-string num) (kbd "RET") reactivate-region)))
    (backward-move-line-or-region (- arg))))

(defun backward-move-line-or-region (arg)
  (interactive "P")
  (if (or (not arg) (>= arg 0))
      (let ((reg-or-lin (if (region-active-p) "'<" "."))
            (reactivate-region (if (region-active-p) "gv=gv" ""))
            (num (if arg (+ arg 1) 2)))
        (execute-kbd-macro
         (concat ":m" reg-or-lin "-" (number-to-string num) (kbd "RET") reactivate-region)))
    (move-line-or-region (- arg))))

These functions probably shouldn't depend on each other, but it's just an idea.

zenobht commented 6 years ago

I have been using the code provided by @aaronjensen for quite some time and it was working really well. But recently it stopped working with the error as user-error: Invalid address. I am not sure if it is caused by emacs 27.0.50 or something in develop branch.

I then found this project https://github.com/rejeep/drag-stuff.el and with below code everything works fine

    (use-package evil
      :ensure t
      :bind (:map evil-visual-state-map
              ;; move visual lines up/down
              ("H-j" . drag-stuff-down)
              ("H-k" . drag-stuff-up)
             :map evil-normal-state-map
              ;; move current line up/down
              ("H-j" . move-text-down) 
              ("H-k" . move-text-up)
            ))

aaronjensen commented 6 years ago

Nice, I'll give this a shot, thanks for the tag :) It must be emacs 27, i'm still on 26 and it's fine.

I noticed this package doesn't indent when you move, but as such it's much faster. I'm probably good w/ that, but maybe I'll update it to indent afterwards some time.

duianto commented 5 years ago

The develop branch has a variable that makes it possible to move lines that are selected with evil char, line or block.

When it's enabled in the user-config:

    (setq vim-style-visual-line-move-text t)

Then J and K moves any evil char, line or block selected lines down or up.

One issue is that the selection changes after moving a char or block selection.

To be consistent, moving selected lines should probably also work with SPC x J and SPC x K: https://github.com/syl20bnr/spacemacs/blob/bd77a5df6e31935a6383fad18ac0d563abf93a8f/layers/%2Bspacemacs/spacemacs-editing/packages.el#L202-L213

and with [e and ]e (evil-unimpaired) https://github.com/syl20bnr/spacemacs/blob/bd77a5df6e31935a6383fad18ac0d563abf93a8f/layers/%2Bspacemacs/spacemacs-evil/local/evil-unimpaired/evil-unimpaired.el#L90-L91

And the selection should probably not change.

github-actions[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid!

stianse commented 4 years ago

To echo @duianto last comment, vim-style-visual-line-move-text t enables the behavior I want. However it took a while before I found that option. A new user will most likely find the transient state option to move text first, and will be surprised when it doesn't work on multiple lines.

duianto commented 4 years ago

I don't know if something's changed, or if I made a typo in my previous comment ☝️.

Because I said that enabling the variable: (setq vim-style-visual-line-move-text t) in the dotspacemacs/user-config section, enables J and K to move the selected line(s).

But when I try it now, by adding it to the dotspacemacs/user-config section and restarting.

Then the keys still call the default evil normal actions:

 J [evil-join]
 K [evil-lookup]

It works from user-init

It does work after setting the variable in the dotspacemacs/user-init section and restarting.

Windows 1903

#### System Info :computer:
- OS: windows-nt
- Emacs: 26.3
- Spacemacs: 0.300.0
- Spacemacs branch: develop (rev. c3f13d039)
- Graphic display: t
- Distribution: spacemacs
- Editing style: vim
- Completion: helm
- Layers:
```elisp
(autohotkey auto-completion command-log emacs-lisp git helm html imenu-list
            (java :variables java-backend 'meghanda)
            javascript latex lsp
            (markdown :variables markdown-live-preview-engine 'vmd markdown-command "vmd")
            multiple-cursors
            (org :variables org-agenda-files
                 '("~/org/notes.org"))
            pdf python
            (shell :variables shell-default-shell 'shell shell-default-height 30 shell-default-position 'bottom)
            spell-checking
            (syntax-checking :variables syntax-checking-enable-by-default nil)
            treemacs version-control)
```
- System configuration features: XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS THREADS LCMS2

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please let us know if this issue is still valid!

duianto commented 3 years ago

The following PRs changes have been applied to the develop branch: Replace move-text with drag-stuff #14449

The text can now be dragged up and down with the commands: M-x drag-stuff-up M-x drag-stuff-down,

Or the key bindings:

 ] e                    ;; drag-stuff-down
 [ e                    ;; drag-stuff-up

The lines can be dragged and the drag stuff transient state can be opened at the same time with:

 SPC x J                ;; spacemacs/drag-stuff-transient-state/drag-stuff-down
 SPC x K                ;; spacemacs/drag-stuff-transient-state/drag-stuff-up

Or the drag stuff transient state can be opened without moving any text, with:

 SPC x .                ;; spacemacs/drag-stuff-transient-state/body

There are also commands for dragging words or selected characters left and right: M-x drag-stuff-left M-x drag-stuff-right

Dragging left and right are included in the drag stuff transient state:

Drag Stuff Transient State
[k/K] up    [h/H] left   [q] quit
[j/J] down  [l/L] right

There's a bug when dragging a word left and right, if the cursor is on the first letter of a word.

It's been reported upstream: bug: drag-left cursor at word[0], drags left neighbour word https://github.com/rejeep/drag-stuff.el/issues/29

syl20bnr / spacemacs