evil-open-above/below ("O"/"o") adds whitespace

[jackkamm]

Description

Adding an empty line with evil-open-below/above ("o"/"O") adds whitespace when it shouldn't.

This issue was fixed in evil in 2012, but appears in spacemacs.

Reproduction guide

• Start Emacs
• put point inside an indented code block (e.g., the dotspacemacs-configuration-layers list in .spacemacs)
• in normal mode, press o, then press esc.

Observed behaviour:
a line containing a sequence of spaces and a '\n'

Expected behaviour:
an empty line which contains only '\n'

System Info

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

(my-spacemacs-layer auto-completion emacs-lisp git org python ipython-notebook ess latex shell syntax-checking eyebrowse)

[bmag]

The indentation behavior of o and O seems to be controlled by the variable evil-auto-indent. A value of nil means "don't indent", and a non-nil value means "indent automatically". The default value is t, so I assume this is an upstream issue. In any case I hope we keep the auto indentation, I find it very useful.

[jackkamm]

I agree that the default value should be to indent. evil adds some hook that removes the extra spaces if "escape" is pressed immediately after "o", i.e. an empty line is opened (according to this it is to mimic the behavior of vim which only adds the indentation lazily). For some reason this hook doesn't seem to be called in spacemacs.

evil-open-below ("o") also seems to behave incorrectly in spacemacs when called in the middle of a collapsed org-mode header, it may possibly be related to this. I will wait to file a separate issue for this though.

[bmag]

For some reason this hook doesn't seem to be called in spacemacs.

Indeed, from a bit of testing I see that the extra indentation is not removed when I'm using hybrid editing style, but it is removed when using vim editing style. Hence this is a bug in Spacemacs and in hybrid mode.

[bmag]

What hook are you talking about, and where does evil add it?

[jackkamm]

Ah, interesting, using "vim" instead of "hybrid" mode also fixed the other error with evil-open-below I was encountering in org-mode headers.

I believe the hook is evil-maybe-remove-spaces, added in evil-append in evil-commands.el.

[jackkamm]

So I believe the trouble is because a custom evil state ("hybrid") is defined in spacemacs for the hybrid-insert-state. In my limited experience, defining new evil states is error prone, lots of things in evil are hardcoded to expect the pre-defined states.

Before spacemacs I was using a simpler hybrid setup, as described here. This doesn't require defining any extra state, instead one just removes all the keybindings from the insert-state.

;; remove all keybindings from insert-state keymap
(setcdr evil-insert-state-map nil)
;; but [escape] should switch back to normal state
(define-key evil-insert-state-map [escape] 'evil-normal-state)

[bmag]

Ok, further research shows that recent changes in Evil caused the definitions of hybrid state and insert state to diverge. Definition of insert state, from evil-states.el:

(evil-define-state insert
  "Insert state."
  :tag " <I> "
  :cursor (bar . 2)
  :message "-- INSERT --"
  :entry-hook (evil-start-track-last-insertion)
  :exit-hook (evil-cleanup-insert-state evil-stop-track-last-insertion)
  :input-method t
  (cond
   ((evil-insert-state-p)
    (add-hook 'post-command-hook #'evil-maybe-remove-spaces)
    (add-hook 'pre-command-hook #'evil-insert-repeat-hook)
    (setq evil-maybe-remove-spaces t)
    (unless (eq evil-want-fine-undo t)
      (evil-start-undo-step)))
   (t
    (remove-hook 'post-command-hook #'evil-maybe-remove-spaces)
    (remove-hook 'pre-command-hook #'evil-insert-repeat-hook)
    (evil-maybe-remove-spaces t)
    (setq evil-insert-repeat-info evil-repeat-info)
    (evil-set-marker ?^ nil t)
    (unless (eq evil-want-fine-undo t)
      (evil-end-undo-step))
    (when evil-move-cursor-back
      (when (or (evil-normal-state-p evil-next-state)
                (evil-motion-state-p evil-next-state))
        (evil-move-cursor-back))))))

Definition of hybrid state, from hybrid-mode.el:

(evil-define-state hybrid
  "Hybrid state for hybrid mode."
  :tag " <H> "
  :cursor (bar . 2)
  :message "-- HYBRID --"
  :entry-hook (evil-start-track-last-insertion)
  :exit-hook (evil-cleanup-insert-state evil-stop-track-last-insertion)
  :input-method t
  (cond
   ((evil-hybrid-state-p)
    (add-hook 'pre-command-hook #'evil-insert-repeat-hook)
    (unless (eq evil-want-fine-undo t)
      (evil-start-undo-step)))
   (t
    (remove-hook 'pre-command-hook #'evil-insert-repeat-hook)
    (setq evil-insert-repeat-info evil-repeat-info)
    (evil-set-marker ?^ nil t)
    (unless (eq evil-want-fine-undo t)
      (evil-end-undo-step))
    (when evil-move-cursor-back
      (when (or (evil-normal-state-p evil-next-state)
                (evil-motion-state-p evil-next-state))
        (evil-move-cursor-back))))))

The changes are related to evil-maybe-remove-spaces, and here is the part of the diff concerning evil-states.el: https://bitbucket.org/lyro/evil/diff/evil-states.el?diff2=f2648b841f9b&at=default

I'll make a PR to update hybrid-state according to insert-state, and I think that would be enough to fix the issue.

[syl20bnr]

Should be fixed in 0.200 right ? I merged the PR before releasing.

[bmag]

yes