Skip to content
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

magit-blame-quit keybinding #2171

Closed
kvaneesh opened this issue Jul 2, 2015 · 17 comments
Closed

magit-blame-quit keybinding #2171

kvaneesh opened this issue Jul 2, 2015 · 17 comments

Comments

@kvaneesh
Copy link

kvaneesh commented Jul 2, 2015

What should be the key binding for magit-blame-quit

@tuhdo
Copy link
Contributor

tuhdo commented Jul 2, 2015

You can alreayd quit with q when in magit-blame-mode.

@kvaneesh
Copy link
Author

kvaneesh commented Jul 2, 2015

didn't work for me with develop branch

@tuhdo
Copy link
Contributor

tuhdo commented Jul 2, 2015

Are you using Emacs or Vim editing style?

@kvaneesh
Copy link
Author

kvaneesh commented Jul 2, 2015

I also have similar issue with magit-show-commit. (not mapped to RET)

@kvaneesh
Copy link
Author

kvaneesh commented Jul 2, 2015

vim

@syl20bnr
Copy link
Owner

syl20bnr commented Jul 3, 2015

You can hit again SPC g b.
Since we are in normal state we have to add q to quit explicitly.
q is working in magit-show-commit for me.

@kvaneesh
Copy link
Author

kvaneesh commented Jul 3, 2015

It would be nice to have a keybinding to do recursive blame. ( I had previously used magit-blame, instead of magit-blame-mode, so repeating SPC g b made it recursive blame). Also at any blame commit, it would be nice to be able to see the full detail of the commit. The blame-mode had that mapped to RET. At any point we want to go back to previous blame state (mapped to 'q' by magit-blame-mode), magit-blame-quit.

@jenanwise
Copy link
Contributor

@kvaneesh I opened a PR to change SPC g b to magit-blame instead of magit-blame-mode in #2213, which will give you recursive blame. q and RET work for me as quit and show, respectively, on develop.

@kvaneesh
Copy link
Author

kvaneesh commented Jul 4, 2015

@jenanwise are you using the vim editing style. With vim style for me neither 'q' and 'RET' is magit features (which is expected). I was expecting them to remapped based on evilify logic.

@kvaneesh
Copy link
Author

kvaneesh commented Jul 4, 2015

btw magit-blame-mode is a minor-mode for cc-mode and my earlier attempt to use evil directly required me to do the below hack

(evil-define-key 'normal magit-blame-mode-map (kbd "q") 'magit-blame-quit)
(evil-define-key 'normal magit-blame-mode-map (kbd "RET") 'magit-show-commit)

(add-hook 'magit-blame-mode-hook
'(lambda ()
(evil-normalize-keymaps)))

Not sure about spacemacs.
I also noticed that C-S-j is not getting mapped as per packages.el.

@jenanwise
Copy link
Contributor

@kvaneesh Vim mode, yes. As of current develop and latest evil, I have to have these in my config:

(evil-make-overriding-map magit-blame-mode-map 'normal)
(add-hook 'magit-blame-mode-hook 'evil-normalize-keymaps)

q, RET, n and P all work. The magit-blame-mode bindings in packages.el are not working for me either.

@kvaneesh
Copy link
Author

kvaneesh commented Jul 5, 2015

@jenanwise , I was not able to add the hook in .spacemacs (It gave error w.r.t magit-blame-mode-map). I could add them to git/packages.el and everything works as you mentioned above. Now shouldn't this hook and overriding-map be part of spacemacs git/package.el ?

@jenanwise
Copy link
Contributor

@kvaneesh Yes you'll have to guard it in something like use-package:

(use-package magit
    :commands (magit-blame-mode)
    :config
    (progn
      (evil-make-overriding-map magit-blame-mode-map 'normal)
      (add-hook 'magit-blame-mode-hook 'evil-normalize-keymaps)))

I'm not familiar enough with the inner workings of evil or emacs keymaps to know whether these should be added to the layer or not. Neither evil-make-overriding-map nor evil-normalize-keymaps are used in any other layer.

There is an active thread in magit/magit#1968 about evil integration with magit. Maybe that will solve our problems?

@TheBB
Copy link
Collaborator

TheBB commented Jul 25, 2015

magit-blame-mode is evilified, like all the other magit modes. That means it you put yourself in evilified state (which is not the same as normal state) you get the intended behaviour with q for quit. (Try by entering evil-evilfied-state after SPC g b.) Evilified modes are automatically added to the list evil-evilified-state-modes, to signify that it should automatically enter this state. However, this only works for major modes… and magit-blame-mode is a minor mode, so evil never sees this.

It's easy enough to enter evilified state automatically:

(add-hook 'magit-blame-mode-hook 'evil-evilified-state)

It's presumably a bigger challenge to return to the state you were in upon quitting.

(defun spacemacs/pre-magit-blame-mode (&rest args)
  (unless magit-blame-mode
    (setq spacemacs--magit-blame-initial-state evil-state)))

(defun spacemacs/post-magit-blame-mode (&rest args)
  (if magit-blame-mode
      (evil-evilified-state)
    (funcall (intern (format "evil-%s-state" spacemacs--magit-blame-initial-state)))))

(advice-add 'magit-blame-mode :before 'spacemacs/pre-magit-blame-mode)
(advice-add 'magit-blame-mode :after 'spacemacs/post-magit-blame-mode)

It's simpler just to return to normal state, maybe.

(add-hook 'magit-blame-mode 'evil-evilified-state)

(defun spacemacs/post-magit-blame-mode (&rest args)
  (unless magit-blame-mode (evil-normal-state)))

(advice-add 'magit-blame-mode :after 'spacemacs/post-magit-blame-mode)

I haven't tested any of these snippets. Maybe @syl20bnr has seen this minor/major/evilified mode problem before?

@synic
Copy link
Contributor

synic commented Sep 25, 2015

Any movement on this? You used to be able to do SPC g b to exit blame, but now it just enters what I like to call blame inception. You have to type iq for every time you have typed SPC g b. It seems to just put another git-blame mode on top of the first.

EDIT: Looks like this is the desired behavior - it blames father back each time you press it. What about binding SPC g B to magit-blame-quit?

synic added a commit to synic/dotfiles that referenced this issue Sep 26, 2015
This will have to do until they fix
syl20bnr/spacemacs#2171
@swsnr
Copy link
Contributor

swsnr commented Oct 14, 2015

I'd like to see this fixed as well. I frequently use magit-blame, and not being able to quit it quickly is a major nuisance.

@synic
Copy link
Contributor

synic commented Oct 14, 2015

I've just been putting this in my user-config:

;; Bind SPC g B to `magit-blame-quit'
(evil-leader/set-key "gB" 'magit-blame-quit)

Also just submitted a PR. I realize that it's probably not the best solution (to me, the best solution would be to make q work in magit-blame-mode), but it's the only one I know how to do, and hopefully it'll get some more eyes on it.

synic added a commit to synic/dotfiles that referenced this issue Feb 17, 2017
This will have to do until they fix
syl20bnr/spacemacs#2171
synic added a commit to synic/dotfiles that referenced this issue Feb 18, 2017
This will have to do until they fix
syl20bnr/spacemacs#2171
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants