Skip to content

Redefining evil-open-above for Haskell (spacemacs) #896

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

Closed
gracjan opened this issue Sep 28, 2015 · 24 comments
Closed

Redefining evil-open-above for Haskell (spacemacs) #896

gracjan opened this issue Sep 28, 2015 · 24 comments

Comments

@gracjan
Copy link
Contributor

gracjan commented Sep 28, 2015

Yet another electric-indent problem:

syl20bnr/spacemacs#3162

@jb55
Copy link

jb55 commented Feb 11, 2016

This is super annoying, any ideas on how to fix? It would be less annoying if you didn't have to press TAB twice...

@gracjan
Copy link
Contributor Author

gracjan commented Feb 11, 2016

The 'twice' part is a misarchitecture in https://github.com/haskell/haskell-mode/blob/master/haskell-indentation.el#L236 . Can be fixed, just somebody needs to sit down and write a specification how this is supposed to work.

@geraldus
Copy link
Contributor

It annoys me too (:

@wraithm
Copy link
Contributor

wraithm commented Feb 15, 2016

I also have similar issues with evil-open-below.

@StreakyCobra
Copy link

I did some research on the Spacemacs issue concerning evil-open-below syl20bnr/spacemacs#5010.

As far as I understood, the function responsible to indent the line after evil-open-* is the function set in the indent-line-function variable, which is set to haskell-indentation-indent-line in this precise case.

Calling haskell-indentation-indent-line on the following line:

foo :: a
|

will indent it "wrongly" to:

foo :: a
       |

I think it's where issues with evil-open-* are coming from, so improving the haskell-indentation-indent-line function should hopefully help with all this 😃

I don't know haskell very well, but I suppose you can improve it to do like python, deducing the information from context. For instance in a signature line end with -> then indent the next line, otherwise not:

foo :: a
|

bar :: a ->
       |

Not sure if it's valid Haskell at all 😄, but you get the idea I hope. FYI Python is doing something like this to have a smarter indentation, by looking at block ender (return, break, pass, …) to know when to indent or not (but in a much more complex way though).

I hope it's helpful.

@fice-t
Copy link
Contributor

fice-t commented Feb 18, 2016

It's valid Haskell, but so is:

foo :: Int
       -> Int
foo = succ

bar :: Either String
       Int -> Either String Int
bar = fmap (+2)

So even without a -> at the end, it still may be valid (unlikely though) to indent it.

@StreakyCobra
Copy link

Ok, but there may be some better approaches though, like using the statistically most probable indentation to diminish the need of using TAB.

I don't have special opinion on this, as I said I'm not doing Haskell, so I let you discuss this inbetween interested people 😃

@fice-t
Copy link
Contributor

fice-t commented Feb 18, 2016

Normally it does just indent to the 0th column. Evil is an exception as it uses (newline) then (indent-according-to-mode) instead of using haskell-indentation-newline-and-indent (RET is bound to that in haskell-mode). I'm not sure why that function is necessary. If one could remove that then that would make things better.

Changing the indentation function when using Evil to something similar to what the above does to:

(defun haskell-indentation-indent-line ()
  "Indent current line, cycle though indentation positions.
Do nothing inside multiline comments and multiline strings.
Start enumerating the indentation points to the right.  The user
can continue by repeatedly pressing TAB.  When there is no more
indentation points to the right, we switch going to the left."
  (interactive)
    ;; try to repeat
  (when (not (haskell-indentation-indent-line-repeat))
    (setq haskell-indentation-dyn-last-direction nil)
    ;; parse error is intentionally not caught here, it may come from
    ;; `haskell-indentation-find-indentations', but escapes the scope
    ;; and aborts the operation before any moving happens
    (let* ((cc (current-column))
           (ci (haskell-indentation-current-indentation))
           (inds (save-excursion
                   (move-to-column ci)
                   (or (haskell-indentation-find-indentations)
                       '(0))))
           (valid (memq ci inds))
           (cursor-in-whitespace (< cc ci))
           (evil-special-command? (and (bound-and-true-p evil-mode)
                                       (memq this-command '(evil-open-above
                                                            evil-open-below
                                                            evil-replace))))
           (on-last-indent (eq ci (car (last inds)))))
      (if (and valid cursor-in-whitespace)
          (move-to-column ci)
        (haskell-indentation-reindent-to
         (funcall
          (if on-last-indent
              #'haskell-indentation-previous-indentation
            #'haskell-indentation-next-indentation)
          (if evil-special-command?
              (save-excursion
                (end-of-line 0)
                (1- (haskell-indentation-current-indentation)))
            ci)
          inds
          'nofail)
         cursor-in-whitespace))
      (setq haskell-indentation-dyn-last-direction (if on-last-indent 'left 'right)
            haskell-indentation-dyn-last-indentations inds))))

should work though. A few Evil users should test this out and see if it works for them.

By itself this means that Evil users would have to press TAB twice like in python-mode to change the default tab position in some cases. This could be fixed somewhat with this:
That shouldn't be a problem with the above version.

@wraithm
Copy link
Contributor

wraithm commented Feb 18, 2016

emacs noob here. Would just dumping this code into my init.el be a good way of testing this code out?

@fice-t
Copy link
Contributor

fice-t commented Feb 18, 2016

You can just paste it into any buffer (scratch is usually for these things), and press C-x C-e at the end of the function(s) if you just want the effects temporarily for one Emacs session.

@wraithm
Copy link
Contributor

wraithm commented Feb 18, 2016

Yeah, just dumped into scratch. Thanks!

My initial impression is that this is much better.

Something sorta interesting in just testing right now, when I'm in the middle of a do expression, it doesn't indent, need to tab to go over.

@fice-t
Copy link
Contributor

fice-t commented Feb 18, 2016

Yeah, that would make sense. I updated the first function. Hopefully it should work this time.

@wraithm
Copy link
Contributor

wraithm commented Feb 18, 2016

Oh wow! This is fantastic.

Just noticed while playing around:

import           Data.Configurator                         (Worth (..), load,
                                                            req^uire)
                 ^

If you do open below on the line with require, it'll indent over. This example works properly:

import           Data.Configurator                         (Wo^rth (..), load)
^

Having the weird hanging import thing confuses it.

@fice-t
Copy link
Contributor

fice-t commented Feb 18, 2016

That seems to be an unrelated bug. haskell-indentation-current-indentation returns 59 when on the require line, which is greater than any indent column (0 and 17 here) of the newline, so it takes the last/greatest one (17) and indents there. This also happens when just pressing RET at the end of the require line. Perhaps you should make a new issue for that (if there's not one already).

Did you find any other issues? Also, I found a way to make it so that Evil guys don't have to press TAB twice anymore. You don't have to use/change the 2nd function now. You can find that version above.

@gracjan
Copy link
Contributor Author

gracjan commented Feb 19, 2016

@fice-t, Nice! Can you create a test suite for this cycle function?

@fice-t
Copy link
Contributor

fice-t commented Feb 20, 2016

I don't know, it might be best if someone with more knowledge in the area than myself would do that. I'm just doing a hack job on top of what's already there.

In other news, I updated the above function with one does away with pressing TAB twice on the last indent position. There's a side effect of the function looking a lot worse, but hey.

Both Evil/non-Evil people should test it out.

Oh, and what should evil-open-above do in this case:

main = do
  something
    where -- cursor is here
      something = putStrLn "Hello" 

Should it indent lining up to the where, or to the something? I changed it back and forth, so I might as well ask to make sure while I'm at this.

@gracjan
Copy link
Contributor Author

gracjan commented Feb 20, 2016

where is expected to be non-empty (although it can be in some contexts).

@fice-t
Copy link
Contributor

fice-t commented Feb 20, 2016

I don't think either case would make it empty.

I just changed it to indent to something. Also note that this only works for Evil when the commands are called by their keybindings, and not through some custom Lisp that calls evil-open-x (well, unless you let-bind this-command to the name of the function around the call). That should be enough for most people.

@gracjan
Copy link
Contributor Author

gracjan commented Feb 21, 2016

@fice-t: Can you create a pull request out of this, with unit tests and whatnot?

@ghost
Copy link

ghost commented Feb 23, 2016

Maybe it is possible to change the implementation of evil-open-below. AFAICT its behaviour should be equivalent to moving point to eol and then pressing return. The reason for the current implementation using indent-according-to-mode is historical: this worked best in Emacs 23. In particular, the command bound to RET is often redefined in modes (or by the user), it might be newline, newline-and-indent or something else. Nowadays electric-indent-mode is another player in that field. It had not been clear what is the most reliable way to get the correct indentation, and the combination of newline with indent-according-to-mode worked quite well for most modes.

In fact, modes with significant whitespace like haskell and python usually make the trouble.

That said, it might be possible (and maybe it's a good idea) to make evil-open-below exactly equivalent to end-of-line followed by (key-binding (kbd "RET")) or so. Maybe we should provide different implementations. There are a few things to consider (like correct handling of the repeat-count) but it should be doable and should not be too hard. It might solve this problem (and related problems with python and the like as well), but we won't know before somebody does the implementation.

Of course, we could tweak haskell-mode, but I would prefer that Evil "just works" without special adaptions. There are no guarantees that Evil's implementation remains as it is now ;)

(I'm the current main developer of Evil)

@fice-t
Copy link
Contributor

fice-t commented Feb 23, 2016

That sounds nice. I edited evil-insert-newline-below to use (execute-kbd-macro (kbd "RET")) instead of (insert "\n"), and that makes evil-open-below work as expected in Haskell. evil-open-above can be 'fixed' in a similar way (plus changing the evil-move-to-beginning-of-line call in evil-insert-newline-above to (evil-move-end-of-line 0) with no forward-line call) to work as well. I had to disable evil-auto-indent for the above to work.

I can imagine this may cause issues elsewhere, but at least it means that it can be done somehow.

Using evil-replace with RET could also be changed in a similar fashion.

@geraldus
Copy link
Contributor

@Lyro thanks for your quick response! (:
I've tested how open-below works in NeoVim and likely it does not make something special, it just enters newline and moves cursor according to previous line, e.g. to first non-blank column. One solution may be copy spaces and tabs from preceding line and paste them after new line char, but this looks like a dirty hack. Another (possibly better) solution will be to check if electric-indent is enabled and execute different actions in different cases: when electric-indent-mode is not present execute newline-and-indent and just enter new line otherwise relying on electric-indent-mode and its customizations in current major mode.

@gracjan
Copy link
Contributor Author

gracjan commented Apr 6, 2016

evil-open-above and evil-open-below in their full glory:

(defun evil-open-above (count)
  "Insert a new line above point and switch to Insert state.
The insertion will be repeated COUNT times."
  (interactive "p")
  (evil-insert-newline-above)
  (setq evil-insert-count count
        evil-insert-lines t
        evil-insert-vcount nil)
  (evil-insert-state 1)
  (add-hook 'post-command-hook #'evil-maybe-remove-spaces)
  (when evil-auto-indent
    (indent-according-to-mode)))

(defun evil-open-below (count)
  "Insert a new line below point and switch to Insert state.
The insertion will be repeated COUNT times."
  (interactive "p")
  (push (point) buffer-undo-list)
  (evil-insert-newline-below)
  (setq evil-insert-count count
        evil-insert-lines t
        evil-insert-vcount nil)
  (evil-insert-state 1)
  (add-hook 'post-command-hook #'evil-maybe-remove-spaces)
  (when evil-auto-indent
    (indent-according-to-mode)))

So it seems that evil-auto-intent: t is equivalent of electric-indent-mode and it will call indent-according-to-mode only on empty lines.

@gracjan
Copy link
Contributor Author

gracjan commented Apr 6, 2016

Closing in favor of #1265 where we will track the real solution.

@gracjan gracjan closed this as completed Apr 6, 2016
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

6 participants