syl20bnr / spacemacs

A community-driven Emacs distribution - The best editor is neither Emacs nor Vim, it's Emacs *and* Vim!
http://spacemacs.org
GNU General Public License v3.0
23.66k stars 4.89k forks source link

Multiple Cursors Integration Discussion #2669

Closed CestDiego closed 2 months ago

CestDiego commented 9 years ago

It's about to get real

This issue is to address how to solve some of the issues on multiple cursors integration in spacemacs. People that are interested shouold comment and discuss what would be the best keybindings and customizations we make on it to make it more..spacemacsy. I'll lay some stuff I've been thinking here:

1.Iedit-state and multiple-cursors have a kind of repeated functionality:

SPC s e is the current multiple editing keybinding and I'd love it if it would stay that way but:

So the questions here is, should we make multiple cursors another layer and make it replace iedit as a whole? or shall we have diferent keybindings for it?

Or should we enforce iedit into vim editing style people and multiple cursors to emacs editing style ones, what happens with the brand new experimental hybrid mode? make it a variable in spacemacs? We know multiple cursors does not play that well with evil keybindings (although it kinda looks like if we set the approapiate .mc-lists.el we might <- shall we explore on that too?

(setq mc/cmds-to-run-for-all
      '(
        electric-newline-and-maybe-indent
        evil-backward-char
        evil-delete-char
        evil-escape-emacs-state
        evil-escape-insert-state
        evil-exit-emacs-state
        evil-forward-char
        evil-insert
        evil-next-line
        evil-normal-state
        evil-previous-line
        forward-sentence
        kill-sentence
        org-self-insert-command
        sp-backward-delete-char
        sp-delete-char
        sp-remove-active-pair-overlay
        ))

(setq mc/cmds-to-run-once
      '(
        helm-M-x
        spacemacs/default-pop-shell
        ))

that's the current one I have, but still I get some errors when I try to use mc and evil normal state to move around.

2. Multiple Cursors overloads the undo tree? or so it seems

After many edits with multiple cursors it seems that the undo tree can't keep up.

3 evil-escape is very bothersome

When using multiple cursors I think that the evil-escape key chord makes emacs wait for the second character when the first has been typed, for example my escape-key is nj so whenever I type n there is a delay there. I think this should replicate for other users with the default f one? We should disable evil-escape mode for multiple cursors maybe?

4. Multiple Cursors Face

Shall it be a line? or a block? this is kinda weird thing to have but it just looks ugly and confusing the more you look at it, check the gif attached to a demo, or just see it for yourself. (but experiment on a scratch buffer, because after a while you wont be able to undo what you've done :P )

alt

robbyoconnor commented 9 years ago

:+1: just following this :) Could have used this (had I known how) in #2662

magnars commented 9 years ago

Please see https://github.com/magnars/multiple-cursors.el/issues/17

From what I can tell, the major hurdle to get multiple-cursors.el to work with evil, is that Emacs and Vim has different views on how cursor positions work.

I don't think anyone with intimate knowledge about these differences between Emacs and Vim has attempted a real fix for this - so sounds to me like you are the best people we have to find a solution.

syl20bnr commented 9 years ago

iedit is superseded by mc so if we get mc right I would replace iedit by mc.

Depending on the editing style mc should behave differently, in holy mode plain mc should be used. In evil mode I would like to apply the same pattern as iedit which is having two states, one to navigate the cursors and activate/deactivate them and another to insert text.

Now that I think about it I wonder how much work it is to extend iedit to operate on overlays with different text. In the current iedit state n and N navigate the cursors and tab toggle their activation, now imaging the possibility to toggle any symbol so you can start by selecting just one symbol (instead of all the occurrences) and then navigate freely in iedit state to toggle other symbols, when you are all set enter iedit-insert state to insert some text. It would be still limited compared to mc but it would allow a lot of new use cases.

syl20bnr commented 9 years ago

So I tried to hack quickly iedit to be able to do this, turns out that there is almost nothing to do to support arbitrary multiple cursors as shown in this video (sorry for the quality): https://vid.me/bcuP

Now it is limited compared to mc but it can already fill additional use cases, maybe it could be sufficient for 80% of the multiple edition needs.

I will investigate a third state for iedit, that is iedit-visual, I don't know if I'll be able to do it though, I remember having difficulty with evil-visual states, but who knows

syl20bnr commented 9 years ago

Reference the inital discussion of mc for spacemacs: https://github.com/syl20bnr/spacemacs/issues/101 (note that despite the Id of the issue - 101 - it has the Challenge tag :-))

(edit: funny to read this thread, iedit integration in spacemacs comes from this issue and a "ground control" mode is mentioned there which is the hole mode we have now)

@magnars are you still using evil ? I believe that you made the switch but I don't know if you sticked with it or not.

syl20bnr commented 9 years ago

I tagged the other issue as duplicated, let's keep this one the official one.

magnars commented 9 years ago

I have tried evil on occasion, since I am quite intrigued by modal editing, but I've got too much weird stuff in my .emacs.d that I'm not willing to let go of or rewrite. Otherwise I'd have much more incentive to really deep dive into why evil and multiple-cursors doesn't play well.

myrjola commented 9 years ago

:+1: I have been looking forward to evil-support after seeing the "Emacs rocks!"-episode.

My biggest concern is with the undo support, the undo tree has become confused every time I have tried to integrate multiple-cursors (mc) with evil. This is a known limitation, what I didn't know that it's usually redo that screws it up. Is this a bug in mc or undo-tree?

@syl20bnr your experiments with iedit are encouraging. The mc-package is very feature-rich, but integrating it with evil has been tried for many years now. Maybe it's easier to implement mc-like functionality to iedit and maybe from those lessons learned we could someday in the future be able to bring evil-support to to mc.

magnars commented 9 years ago

The mc-package is very feature-rich, but integrating it with evil has been tried for many years now.

While it has indeed been years, I don't think the effort put into it is in that order of magnitude. I'd be happy to assist with my knowledge of the inner workings of multiple-cursors, if someone on this team wants to give it a shot.

myrjola commented 9 years ago

I could give it a shot. My elisp skills has increased somewhat since I tried multiple-cursors the last time. I'll start by reading some code and experimenting. @magnars is there any documentation of the inner workings of mc or is there certain sections you can refer me to? I think the first problem to solve would be the cursor position differences between Emacs and Evil.

magnars commented 9 years ago

Great!

There are quite a few files in multiple-cursors, but all the magic happens in multiple-cursors-core.el. I've done my best to document all the functions. In short, multiple-cursors simulates the Emacs command loop. It registers a post-command-hook, and then runs through the last command for all fake cursors.

Take a look at core, and ping me back with more questions when you have them. :)

On Mon, Aug 17, 2015 at 6:41 PM Martin Yrjölä notifications@github.com wrote:

I could give it a shot. My elisp skills has increased somewhat since I tried multiple-cursors the last time. I'll start by reading some code and experimenting. @magnars https://github.com/magnars is there any documentation of the inner workings of mc or is there certain sections you can refer me to? I think the first problem to solve would be the cursor position differences between Emacs and Evil.

— Reply to this email directly or view it on GitHub https://github.com/syl20bnr/spacemacs/issues/2669#issuecomment-131885267 .

myrjola commented 9 years ago

Time for a progress report. I have done some reading in multiple-cursors and evil sources. Still wrapping my head around all the things that need to be done.

I started to solve the problem with evil-state-transitions not moving the cursor properly in the fake cursors. You can reproduce this by having the evil-move-cursor-back-variable in the default value t and alternating evil-normal-state and evil-insert-state with multiple cursors enabled. You should see that the point moves backwards when exiting the insert state, but the fake cursors don't move.

The function doing the movement is also named evil-move-cursor-back. The backtrace looks like this when evil-normal-state is called with multiple-cursors active:

    evil-move-cursor-back()
    evil-insert-state(-1)
    evil-change-state(nil)
    evil-normal-state()
    call-interactively(evil-normal-state nil nil)
    command-execute(evil-normal-state)

The evil-change-state-function handles the state transitions by comparing the evil-state-variable to which state we are moving to. The problem is that the state transition is done only once when we would like to be able to have the same transition for each cursor.

Note from the backtrace that we haven't entered any multiple-cursors code yet, so I can't come up with a nice way to inject what we need. However there's some possible solutions to the problem:

  1. (setq evil-move-cursor-back nil) now state transitions doesn't move the cursor. However this is a non-standard configuration, so many people will be confused if we change that variable when enabling multiple-cursors.
  2. Introduce a mc/evil-change-state-function that ensures a proper state transition for every cursor.
  3. Somehow reset to the previous state between command executions for each cursor.
  4. Having a special rule in multiple-cursors that takes this situation into account, probably not a good idea.

What do you think?

TheBB commented 9 years ago

Can you advise evil-change-state to do the right thing™?

magnars commented 9 years ago

Excellent detective-work, @myrjola.

  1. doesn't feel like a happy solution to the problem.
  2. How would mc/evil-change-state work? Maybe evil-change-state offers a hook? In that case, mc/evil-change-state would have to reproduce what happens to each cursor in evil-change-state. Sounds workable. Might have to occasionally keep up with master tho.
  3. This might actually work. If the evil-state is stored in a variable, you can add it to the mc/cursor-specific-vars list, and multiple-cursors will keep track of them on a per-cursor basis. Maybe evil-change-state is actually run per cursor already by the command loop imitator, but since the evil-state is already changed, it does not run for every cursor. This relies on there not being a lot of other side effects when changing states.
  4. I'm not opposed to the idea of giving evil some special treatment in multiple-cursors.
Kethku commented 9 years ago

I evaled (push 'evil-state mc/cursor-specific-vars) and I think it fixed the problem. Repeatedly pressing i and esc causes all of the cursors to move back one character at a time. Seems to be better!

Similarly pressing v with 'evil-state added to the list causes all of the cursors to go into visual mode as apposed to just every other one, however pressing escape causes the cursors to get stuck. Haven't done much investigation with this issue though. Could very well be that I accidentally skipped one of the messages to track escape which would do it.

On that subject, we should probably build up a list of commands that pre populate the tracked commands list so that it doesn't keep asking when you first try multiple cursors.

A tad off topic, but I think it might be very valuable to disable micro states while in multiple cursors mode. For instance pasting repeatedly while in mc mode causes the extra cursors to go away and as far as I can tell it is due to the paste micro state.

myrjola commented 9 years ago

Thank you @Devagamster for your experiments! I think that using mc/cursor-specific-vars is the cleanest solution. I'll start investigating that visual-state problem today.

The micro states can be a difficult problem to solve as there's probably much state involved. Maybe we would need to record all the key-presses in a micro state and after exiting the micro state run the recorded macro for all cursors. I'll mark it up in my issue list.

@magnars I think we could start proposing pull requests to multiple-cursors soon. Do you prefer one PR for each problem or a more general evil-compatibilty PR where I'll gather all the commits? I think the evil-compatibility PR would be cleaner and easier to merge once it has got enough testing.

magnars commented 9 years ago

I agree that a single evil-compat PR is best. tor. 20. aug. 2015 kl. 08.27 skrev Martin Yrjölä notifications@github.com:

Thank you @Devagamster https://github.com/Devagamster for your experiments! I think that using mc/cursor-specific-vars is the cleanest solution. I'll start investigating that visual-state problem today.

The micro states can be a difficult problem to solve as there's probably much state involved. Maybe we would need to record all the key-presses in a micro state and after exiting the micro state run the recorded macro for all cursors. I'll mark it up in my issue list.

@magnars https://github.com/magnars I think we could start proposing pull requests to multiple-cursors. Do you prefer one PR for each problem or a more general evil-compatibilty PR where I'll gather all the commits? I think the evil-compatibility PR would be cleaner and easier to merge once it has got enough testing.

— Reply to this email directly or view it on GitHub https://github.com/syl20bnr/spacemacs/issues/2669#issuecomment-132905645 .

Kethku commented 9 years ago

Found another simple fix.

Problem: If you have multiple cursors but have them staggered across different columns, repeatedly moving up or down a column will cause all of the cursors to jump to the same column instead of preserving their current horizontal position.

Fix: Add temporary-goal-column to mc/cursor-specific-vars with (push 'temporary-goal-column mc/cursor-specific-vars)

Kethku commented 9 years ago

Just to add to the list, another big issue has to do with the fact that most operators do not work. This includes deleting with d and changing with c off the top of my head. I think it has to do with a similar state management issue as above but the symptoms are weird. Instead of just missing some bit of the state, all of the cursors disappear.

I have a hunch that it happens because every cursor is moved to the same position as the main one causing MC to prune the cursors leaving none remaining, so it is quite possible that it will just require adding another variable to the mc/cursor-specific-vars list, but I haven't had the time to track down exactly which one is causing the problem because the offending function (at least I think it is the problem) is the evil-define-operator macro in evil-macros.el and is 80 lines long.

When I get the chance I am going to try adding the evil-operator- variables to the tracked list and see how that goes. A tad trial and error but w/e.

myrjola commented 9 years ago

One problem I noticed with adding evil-state to mc/cursor-specific-vars is that when starting mc in insert-state doesn't let me enter normal state anymore. I fixed this by adding some related variables to mc/cursor-specific-vars.

evil-previous-state      
evil-next-state          
evil-previous-state-alist

The visual state and operator problems still eludes me, it seems that the region passed to the operators is wrong. For example choosing a region in visual state with multiple cursors and issuing the evil-delete-operator with d will pass a one character wide region for the beg and end parameters for the operator regardless of how wide region i have selected in visual state.

I'm sure we can get many more problems fixed if we continue by adding more variables to mc/cursor-specific-vars. Hopefully all of them :pray:.

Kethku commented 9 years ago

If all of the solutions can be fixed using mc/cursor-specific-vars, then it is possible a pull request to mc would not be necessary. We could just add a default .mc-lists.el to spacemacs and set the mc/cursor-specific-vars on startup.

syl20bnr commented 9 years ago

Keep up the good work guys :+1:

I let a message on the evil issue tracker to ask for some help from the maintainer of Evil.

For the micro-states issue, for now we can ignore them, we will see later how to handle transient keymaps.

syl20bnr commented 9 years ago

I wonder if all the work done to isolate the memory state involved in Evil commands could in the long term be beneficial to the Evil project to improve its implementation, for instance some kind of context object could be passed to Evil in each command so Evil could indirectly support multiple cursors in its core and multiple-cursors could be the cursor manager on top of it.

It would be more clean and futureproof.

Kethku commented 9 years ago

I did a crazy thing. I built a list of all of the variables in spacemacs that start with evil- (turns out there are 435) and added them all to the mc/cursor-specific-vars just to see what gets fixed and what breaks. Turns out the operator bug is not fixable by adding an evil- variable to the list. Surprisingly it runs fairly well even with so much state, but unfortunately it didn't fix everything. It is possible that a different variable outside of evil could be the culprit however, but it does cross off a large portion of the variables we have to look at.

myrjola commented 9 years ago

@Devagamster, what a bummer that operators didn't get fixed with those cursor-specific-vars. We need to get more clever now.

I finally had some time to investigate this further. I started looking at how operators work. Let's take evil-delete as an example.

(evil-define-operator evil-delete (beg end type register yank-handler)
  "Delete text from BEG to END with TYPE.
Save in REGISTER or in the kill-ring with YANK-HANDLER."
  (interactive "<R><x><y>")

The interactive specification is key here. There's evil-specific codes in the arguments section, the <R> code means that the operator takes a typed range.

(evil-define-interactive-code "<R>"
  "Typed motion range (BEG END TYPE)."
  (evil-operator-range t))

If I understood the code correctly the type is used for special cases like block selections. evil-operator-range reads the motion from the keyboard and the resulting typed motion range (beg end type) is passed to evil-delete. For multiple-cursors use we would like to read the motion only once and execute every call to evil-delete with that motion. I'll see if this can be achieved somehow elegantly.

One interesting note regarding instrumenting functions with edebug. When instrumenting evil-delete and having two cursors active shows that evil-delete is entered only once when pressing de and so delete to end of word is executed only for the main cursor, but not for the fake cursor. Instrumenting evil-operator-range however changes things. Now the motion e is read twice from the keyboard and evil-delete is executed properly for both cursors. Maybe multiple cursors has a feature that inhibits the reading of keypresses when executing commands for the fake cursors and somehow edebug overrides that functionality.

myrjola commented 9 years ago

Here's what I came up with for enabling operator-motion support:

(defvar mc--evil-motion-read-results nil
  "Stored results from the last call to `evil-motion-read'. The
  results are stored so that a motion doesn't need to be read by
  each fake cursor.")

(defun mc/evil-read-motion-advice (orig-fun &optional motion count type modifier)
  (if mc--executing-command-for-fake-cursor
      mc--evil-motion-read-results
    (let ((res (apply orig-fun motion count type modifier)))
      (setq mc--evil-motion-read-results res)
      res)))

(advice-add 'evil-read-motion :around #'mc/evil-read-motion-advice)

The problem is that it doesn't work without instrumenting or calling (debug) inside evil-operator-range. It's the same peculiarity that I mentioned in my previous comment and I'm stumped. I can't figure out why trying debugging makes the adviced functions work in some cases. So here's what results I get, would be nice if anyone can confirm the same behaviour:

The cursors are represented by '!'. Start with the buffer:

as!df asdf
asdfasdf

Execute mc/mark-next-like-this.

as!df asdf
as!dfasdf

Press de, that is "delete to the end of the word". I expect this:

as! asdf
as!

But I get this:

as! asdf
asdfasdf

Notice that the fake cursor also has disappeared. Now if you try the same but have instrumented mc/evil-read-motion-advice or evil-read-motion you can see that the function is entered only once, i.e. the fake cursor is skipped. This can also be proved by printing debug statements with message. But now to the mystery part: If you instrument either evil-operator-range or add a call to debug somewhere inside evil-operator-range everything works as expected. Any ideas why this happens?

I'll sleep on this and try next time with a minimal Emacs installation to reproduce to make sure nothing else is at play. Alternatively I will start thinking about the state encapsulation @syl20bnr talked about in https://github.com/syl20bnr/spacemacs/issues/2669#issuecomment-133157594. I too think that would be beneficial for Evil and make our work easier. I'll start with trying to encapsulate the operator-motion related state. Is EIEIO-classes recommended for this kind of encapsulation?

Kethku commented 9 years ago

I almost wonder if EIEIO classes would be overkill. If the state is already made up of mostly global variables (as it seems to be) then a simple association list could probably work just fine. I don't know if the problem is more complicated than that though. I have been shying away from deep diving into evil-mode just because its not on github and I've been busy. I may take a deeper look at the debug issue you've been encountering next chance I get some free time.

myrjola commented 9 years ago

I solved the mystery from https://github.com/syl20bnr/spacemacs/issues/2669#issuecomment-136178913! Consider these lines from the evil-operator-range function.

            ;; Make linewise operator shortcuts. E.g., "d" yields the
            ;; shortcut "dd", and "g?" yields shortcuts "g??" and "g?g?".
            (let ((keys (nth 2 (evil-extract-count (this-command-keys)))))
              (setq keys (listify-key-sequence keys))
              (dotimes (var (length keys))
                (define-key evil-operator-shortcut-map
                  (vconcat (nthcdr var keys)) 'evil-line)))

The call to this-command-keys fails silently when a command is executed for a fake-cursor. The exception to this is when the evil-operator-range function is instrumented with edebug-defun or includes calls to (debug). Next I have to dive into the C source to see if anything can be done about this.

@magnars this finding could be interesting for multiple-cursors overall. If a package makes use of the builtin this-command-keys C-function there's a chance the package doesn't work well with multiple-cursors.

myrjola commented 9 years ago

I think it's best to keep the discussion nearer to the code. Let's continue in the PR.

syl20bnr commented 9 years ago

There is another attempt to bring multiple cursors to evil users, I tested it quickly and it seemed more stable than the current attempt in multiple-cursors.el. I added it in develop branch in commit https://github.com/syl20bnr/spacemacs/commit/31229cb1f5a556a1d368658aa52b0d8482d1c04e (see commit message). Happy testing :-)

myrjola commented 9 years ago

I did some limited testing and also think this is more stable than my hacking in https://github.com/magnars/multiple-cursors.el/pull/216. I very much liked the helpful error messages to show that e.g. evil-visual-mode was not supported instead of exploding in my face.

I'm terribly busy at the moment, so it will take some time to get the PR for multiple-cursors to a mergeable state. If it shows that the community wants to use evil-mc instead, then my time is better spent elsewhere.

One thing that's not optimal is the duplicate effort in evil-mc and multiple-cursors. I haven't yet taken a proper look at the evil-mc code, but it seems that the architecture is built with evil in mind. It will probably be hard to merge these projects together. I really like all the "mark-like-this" commands in multiple-cursors. If there would be some way to encourage code reuse I would be all for it.

cpaulik commented 9 years ago

@myrjola I've just quickly tested evil-mc and it does have commands like evil-mc-make-all-cursors that makes a cursor on each match. Or evil-mc-make-and-goto-next-match for making cursors on a subset of matches

myrjola commented 9 years ago

@cpaulik, yeah the basic functionality is there. But multiple-cursors does have a plethora of useful commands that I would like to use. I'm especially a fan of mc/mark-all-like-this-dwim.

syl20bnr commented 9 years ago

While it is a duplicated effort, it is a needed duplication to this issue so we get a better understanding of what need to be done. It may happen that if evil-mc works in the end then emacs-state is supported out of the box making it a potential drop-in replacement for multiple-cursors. Anyway Spacemacs relies on Evil foundation so from my point of view having a rewrite of multiple-cursors on this foundation is very appealing.

syl20bnr commented 9 years ago

cc @gabesoft

magnars commented 9 years ago

It doesn't shed as much light on the issue of "what needs to be done in multiple-cursors", since evil-mc has added explicit support for a lot of evil-specific commands. multiple-cursors.el does not add any such helpers, instead trying to fully replicate the emacs command loop to work for all commands - whichever package it is in. It saddens me a little that the general approach is abandoned with this change, but I see that evil might be a special enough case to warrant it.

gabesoft commented 9 years ago

@myrjola I think that after evil-mc becomes more stable we will be able to extract the common functionality shared with multiple-cursors. Most of the commands for creating cursors that multiple-cursors has should exist in evil-mc. Creating cursors turns out to be the easy part. What's more difficult is keeping all fake cursors in sync with the real cursor and having them work properly for all evil commands.

syl20bnr commented 9 years ago

since evil-mc has added explicit support for a lot of evil-specific commands

Seems to me a viable reason to have another thread of research in this domain.

gabesoft commented 9 years ago

@magnars I would love nothing more than to be able to unify all commands. But I wasn't able to find a way, partly due to me being new to emacs and elisp, and partly due to the complexity of some evil commands.

edrex commented 8 years ago

Just surfacing TheBB's https://github.com/syl20bnr/spacemacs/issues/5324#issuecomment-191104615 with a more selective method for enabling evil-mc-mode, which avoids a keybinding conflict with magit, for people testing it out:

(add-hook 'prog-mode-hook 'turn-on-evil-mc-mode)
(add-hook 'text-mode-hook 'turn-on-evil-mc-mode)
vdm commented 8 years ago

vis has a decent well-integrated implementation of multiple cursors which may be of interest. https://github.com/martanne/vis#multiple-cursors--selections

lionel- commented 8 years ago

The branch in https://github.com/magnars/multiple-cursors.el/pull/216 has become quite usable with the last commits.

royalaid commented 8 years ago

Just curious as to the status of this

anonymni commented 8 years ago

There is also a new one which might be interesting to look at.

https://github.com/mm--/ace-mc

xetra11 commented 8 years ago

Go get it!

trishume commented 8 years ago

Unfortunately it seems for me the backspace key in insert mode only does anything on the main cursor, making everything get out of sync.

Steps to reproduce:

aaronjensen commented 8 years ago

@trishume are you using hungry delete mode? If so, try disabling it.

jtamagnan commented 7 years ago

The face issue has recently(ish) been fixed the the following face: mc/cursor-bar-face

autosquid commented 7 years ago

@aaronjensen disabling hungry-delete-mode seems helps a little, but I still could not get the right result if I once move the curve with h l in visual mode. Do you have the same problem?

aaronjensen commented 7 years ago

@autosquid could you give a more detailed repro? I'm not sure I understand what you are asking.