Closed justbur closed 8 years ago
@tarsius what's the easiest way to move popup commands?
@justbur magit-change-popup-key
.
I tried this yesterday. Let me be more specific. :-P
(magit-change-popup-key 'magit-dispatch-popup :action ?k ?K)
For example. The popup argument is the symbol of the function that triggers the popup.
@TheBB I can give you access to my branch if you want to help :-P On Tue, Oct 27, 2015 at 9:51 AM Eivind Fonn notifications@github.com wrote:
I tried this yesterday. Let me be more specific. :-P
(magit-change-popup-key 'magit-dispatch-popup :action ?k ?K)
For example. The popup argument is the symbol of the function that triggers the popup.
— Reply to this email directly or view it on GitHub https://github.com/magit/evil-magit/pull/2#issuecomment-151502567.
Sure, why not.
@TheBB ok you should have access now. I'm just on master. I added a TODO list in the header.
@tarsius For the popups, it seems like magit-dispatch-popup
might be the only one where the keybindings need to change to be consistent with the bindings in the magit buffers here. Are there any other popups that you know of that have bindings that would need to change?
To give an example, magit-log-popup
has a binding for magit-log-head
(bound to h
), but that's ok with me because there's no other way to reach magit-log-head
outside of the popup. I'm looking for commands that have one key bound to them in a buffer but another in a popup.
@tarsius After a bunch of tweaks, I'm pretty happy with this now (I figured out the popup thing I think). I won't touch it for now if you want to review it or invite others to. Let me know.
@mbertheau cc'ing you as another person I know who may provide feedback
@justbur Thanks for the heads-up. I want to try it but I'm not familiar enough with emacs to know how to install and use this package. I'll try checking out evil-magit in my elpa directory and including it in the package list of the git layer.
From what I can see here I'm very excited!
@mbertheau clone the repository and use this in your init file
(with-eval-after-load 'magit
(load "/path/to/evil-magit/evil-magit.el"))
@justbur Thanks! I can report initial happiness. If anything comes up during the next days I'll comment here. Thanks for the nice work!
Seems like I can't do selective staging of lines without reaching to the arrow keys after entering visual selection mode with V
j,k
should work when in visual selection mode for selecting lines for staging/discarding. Current behaviour is, it jumps by hunks. Neither C-j
, C-k
work which are bound to visual line selection work as expected until I enter visual selection mode.
C-n
C-p
seem to be conflicting with evil-mc
. Disabling evil-mc
didn't help either with C-n,p
to select visually which seems to be bound to evil-paste-pop{-next}
.
All above tested on Spacemacs.
@nixmaniack fixed in 4f4783f
The intention is to use C-j
and C-k
for line by line movement everywhere
@justbur Works as expected now. Thanks for the quick fix.
@justbur Why have you chosen to create dispatch functions that shadow vanilla magit bound functions ? I moved away from this initial implementation in Spacemacs for evilified buffer to replace it with a pre-command-hook and a submap (https://github.com/syl20bnr/spacemacs/blob/develop/core/core-evilified-state.el#L62-L69) so I wonder what benefits are lost with this change.
@syl20bnr I'm not sure one is better than the other. I guess I wasn't thinking of this as a mode or a state with hooks that need to run to activate and deactivate things like pre-command-hooks. Does that make sense?
It has not to be a state, any submap could do it, actually the evilified state allows to leverage the evil API for binding definitions but it could have been done manually, the dispatch is still manually done in a custom pre-command-hook.
I believe it is cleaner to have a tiny central place that acts as the dispatcher than replicating the work in every functions but I'm not sure about the side effects of this because I'm not aware of the whole command loop of Emacs, this is why I asked you if you have benefits that we may loose in auto-evilification.
I'm hijacking this PR so we can continue the discussion in the Spacemacs thread if you want.
@syl20bnr I'm not sure this is the best implementation. I was more concerned with making good choices for key bindings to start, getting something that works, and then fixing up the implementation later.
You are right, make it work first :+1:
Can we include [ and ] as bindings as well to navigate sections? Also from this, I cant see if there is an option to visually select and stage region instrad of entire hunk. Since j/k are mapped for navigating sections would we perform be using c-j and c-k to move the cursor around?
Sure why not On Mon, Nov 2, 2015 at 11:52 AM Rich Alesi notifications@github.com wrote:
Can we include [ and ] as bindings as well to navigate sections?
— Reply to this email directly or view it on GitHub https://github.com/magit/evil-magit/pull/2#issuecomment-153080941.
@ralesi 40c674b
I find the remapping of j
and k
to jump between sections very confusing since now magit-status
is pretty much the only buffer where I can not use j
and k
like I normally would. The same applies for selection in visual mode where this is the only mode where I have to do v
and then C-j
and C-k
instead of the normal keybindings.
@cpaulik j and k do what n and p do in the magit bindings. Are you suggesting?
C-n -> j
C-p -> k
n -> C-j
p -> C-p
Are there any other popups that you know of that have bindings that would need to change?
Cannot think of any, but for popups which bind the same key to the "default action" which is used to enter the popup, you probably want to keep it that way (i.e. change both bindings).
Also sorry for not replying earlier. I just cannot contribute much at this point (also I am taking a break now, so won't be contributing anything for a while). One think I would like to note though, is that I was hoping for something a little simpler. Just saying in case someone else wants to contribute another implementation (whether that be a simpler implementation or one that just takes a different approach).
Yes, since normal emacs bindings for next and previous line (C-n
and C-p
) stay unchanged in magit I would also leave them unchanged for evil.
Going through sections should be something special like moving by paragraphs. An the gj
and gk
bindings are already used in this way.
@cpaulik ok makes sense to me On Thu, Nov 5, 2015 at 8:35 AM Christoph Paulik notifications@github.com wrote:
Yes, since normal emacs bindings for next and previous line ('C-n' and 'C-p') stay unchanged in magit I would also leave them unchanged for evil.
Going through sections should be something special like moving by paragraphs. An the gj and gk bindings are already used in this way.
— Reply to this email directly or view it on GitHub https://github.com/magit/evil-magit/pull/2#issuecomment-154059471.
Actually I think I agree with @cpaulik too on that one. As in, if it's not done here, I probably have would remapped anyway so that the normal state evil mode j
and k
(i.e. move down/up a line) bindings stay consistent with everything else. How does everyone else feel about it?
Yes, jk
would make more sense for current C-jk
.
@tarsius how would you fee about me making this into a separate Melpa package and letting it mature? I'd be happy to give it back in the future
I'll flip the jk bindings in a little bit On Thu, Nov 5, 2015 at 8:53 AM Muneeb Shaikh notifications@github.com wrote:
Yes, jk would make more sense for current C-jk.
— Reply to this email directly or view it on GitHub https://github.com/magit/evil-magit/pull/2#issuecomment-154063614.
@cpaulik @vyp @nixmaniack I changed the behavior of j and k. Let me know what you think
I obviously like the j
and k
behavior :smile: . Why did you remove the C-jk
bindings instead of switching them like the commit message says? I think C-jk
would be good keybindings as an alternative to g jk
or []
@cpaulik I can bring them back. What command do you want them on?
@justbur I would say magit-section-forward
and magit-section-backward
.
That's what the commit said actually, Switch role?
@cpaulik @nixmaniack The commit message is wrong, sorry (I'll revert it next time I push). Instead I put gj
and gk
on magit-section-forward
and magit-section-backward
respectively. @cpaulik Do you want C-j
and C-k
on those too. There are also commands for jumping further called magit-section-forward-sibling
and a backwards version. I have [
and ]
on those now, per @ralesi's suggestion. I'm happy to do whatever you guys want with these, but it would be nice to have a little consensus.
I like to use jk
the same way I use it in other buffers when in normal mode. And C-jk
is good for magit-section-{forward,backword}
.
On a side note, I think I had reported similar issue when in status section where I'd expected hjkl
to work the normal vim way.
@nixmaniack Default magit only has movement by line in most buffers, and I follow that here. If I remember right, your previous issue was with not being able to move right and left in the status buffer to copy text.
Yes, that's right. It was resolved with C-jk
which is what was the expected behaviour at that time. But just mentioning as I wanted jk
there as well, so that we can have consensus on jk
for visual line movement and have C-jk
for magit section navigation.
@justbur I would like C-jk
to move by sections because it is easier to type than gj
and gk
when browsing through several sections. And also easier than []
on a :de: keyboard.
@cpaulik @nixmaniack You're only answering part of my question. Perhaps I wasn't clear. There are two types of section movement magit-section-forward
and magit-section-forward-sibling
. The second usually jumps further. Since I don't have a strong opinion on this, I'd like to know which keys should go to both commands. The table at the beginning of this issue shows the current state. I'd like to come up with better movement commands (based on your recommendation) and make them consistent across all the maps.
Right now, I hear you say C-jk
should be the shorter magit-section-forward
and magit-section-backward
. Would you have any other keys for those commands? What would you put on the longer jumping magit-section-forward-sibling
and magit-section-backward-sibling
?
This is what is expected right now. "j" evil-next-visual-line "k" evil-previous-visual-line "\C-j" magit-section-forward "\C-k" magit-section-backward
Since I don't use magit-section-{forward,backward}-sibling
(that often), but if you ask for suggestion M-jk
might be the next better spot as in magit they're on M-np
iirc.
I think gj and gk are good fits for the -sibling commands.
@cpaulik @nixmaniack pushed the changes. Refer to the table at the beginning of the issue. I think you will both be happy
cc @TheBB @syl20bnr
I'm not done with this, but this is at least a starting point. I think some of the choices need some work. I didn't know where to put rename for instance. I also need to fix the popups.
Please give feedback on what you'd like to see changed. Here's a table showing the basic ideas
Other/Additions