Unable to stage individual lines using magit

[dankessler]

Description

Unable to stage individual lines using magit

I believe this is due to this recent change in magit.

Previously, magit-stage was bound to s using the keymap text property,
which has very high priority. Now, instead the text property simply rebinds
magit-stage-file to magit-stage. s is bound to magit-stage-file in the
keymap for the magit-status global mode, but evil-surround binds s to
evil-surround-region when in visual state, which has higher priority than the
global mode keymap.

This isn't really a bug in magit but rather a complication of using
evil-surround and magit at the same time. This could potentially be remedied
by making a change to evil-collection to explicitly map s to magit-stage
in a way that would override the evil-surround-region mapping, or really any solution that would appropriately over-shadow the evil-surround-region mapping with magic-stage. I'm happy to take this on in a PR, but am open to input on the most graceful way to fix this (e.g., in spacemacs, in evil-collection, or (somehow?) in magit itself).

Reverting to an older version of magit remedies the issue.

Reproduction guide 🪲

• Start Emacs
• Edit some lines in a file under version control
• Open a magit status buffer (SPC g s)
• Move point to a line in the diff hunk
• Change to visual mode (v) and select a line or two (j)
• Hit s to try to stage just those lines

Observed behaviour: 👀 💔
s doesn't stage and instead waits for another character due to evil-surround-region firing

Expected behaviour: ❤️ 😄
s should trigger magit-stage and stage the currently selected lines

System Info 💻

• OS: darwin
• Emacs: 27.2
• Spacemacs: 0.999.0
• Spacemacs branch: develop (rev. d79b12e)
• Graphic display: t
• Distribution: spacemacs
• Editing style: hybrid
• Completion: helm
• Layers:

((auto-completion :disabled-for org)
 better-defaults bibtex csv emacs-lisp
 (ess :variables ess-assign-key "\255")
 eww git
 (helm :variables helm-use-fuzzy 'source)
 html
 (latex :variables latex-enable-auto-fill nil latex-enable-folding t latex-enable-magic nil latex-refresh-preview t)
 lsp major-modes markdown osx
 (org :variables org-adapt-indentation nil spaceline-org-clock-p t org-want-todo-bindings nil)
 outshine pdf python
 (shell :variables shell-default-height 30 shell-default-position 'bottom)
 spacemacs-navigation spell-checking syntax-checking
 (version-control :variables version-control-global-margin nil)
 (treemacs :variables treemacs-use-scope-type 'Perspectives treemacs-use-follow-mode nil treemacs-recenter-after-tag-follow 'on-distance))

• System configuration features: NOTIFY KQUEUE ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS JSON PDUMPER GMP

[practicalli-johnny]

After a package update today, I experience the same issue. I regularly use this feature of magit, so hope someone can contribute a work-able fix to Spacemacs to make these packages work again.

Rather than do a full package update rollback, I manually copied the previous magit package folder, magit-20220305.1056 from the rollback directory
~/.emacs.d/.cache/.rollback/29.0/develop to the elpa directory ~/.emacs.d/elpa/29.0/develop
and deleted the newly downloaded magit package.

A full rollback is a safer approach though.

[lebensterben]

So maybe it's an upstream issue?

[dankessler]

It is indeed upstream in a literal sense. However, it's arguably not a bug in magit (since moving keybindings from text properties to a major-mode map is a design choice), nor is it really a bug in evil-surround (since evil-surround binds the s key in a idiomatic evil way). Instead, it's really just an unfortunate interaction that arises from using both at the same time. In this sense, it is spacemacs' fault, since spacemacs turns on evil-surround globally, which includes modes where it doesn't really make sense to use it (e.g., in a magit status buffer). The simplest fix is probably to selectively disable evil-surround in modes where it's of no use (and only likely to get in the way), which is a change that should be effected in spacemacs.

[lebensterben]

to fix this:

• add a function git/post-init-evil-surround to git layer
• within it, add a :post-config hook for magit, such that magit status buffer turns off evil-surround on creation.

[dankessler]

Perfect; I'll do that in a PR shortly

[lebensterben]

I also think it makes sense not to turn on evil-surround globally.

Specifically, it can be turned on for all text-mode and prog-mode. Or in other words, specialm-mode won't be affected.

This should be fine in most use cases. Except for a few specfic special-mode such as various terminal, inferior emacs lisp, inferior python, etc. But then evil-surround can be turned on when needed.

[practicalli-johnny]

With the latest Magit Melpa package installed, using SPC SPC evil-surround-mode in the magit status buffer allows staging of a visually selected region within a hunk.
So only turning evil-surround for text-mode and prog-mode seems a workable approach to me.

[dankessler]

Addressed this in #15462. While I agree that evil-surround probably isn't useful outside of modes derived from text-mode or prog-mode, I can't say with 100% confidence that this is the case, so I opted for a much lighter touch and only disabled it in magit status buffers to avoid irritating anyone who is using evil-surround in ways that we aren't thinking of.

[dankessler]

Fixed in #15462

[lebensterben]

I will keep it open and close it until evil-surround is turned off for special-mode buffers.

[dankessler]

I will keep it open and close it until evil-surround is turned off for special-mode buffers.

Why do that though? Is it causing any issues as far as you know?

[lebensterben]

@dankessler

Okay I made a tracking issue referencing this and closed this one.