vim-erlang / vim-erlang-runtime

Erlang indentation and syntax for Vim
https://vim-erlang.github.io
101 stars 29 forks source link

Indent after a line with trailing = #28

Closed evnu closed 9 years ago

evnu commented 9 years ago

In OTP, indentation after = is handled similar to this example:

f(X) ->
    MyVariable =
        case X of
            true -> ok;
            false -> still_ok
        end,
    MyVariable.

This PR implements indentation after a trailing = (with possible whitespace). When a trailing = is encountered, the following line is indented by one shiftwidth of the previous line.

I chose to implement this nearby the implementation of handling comments, as I guessed that this would be the proper way to do this. If the logic should be placed somewhere else instead, please let me know.

hcs42 commented 9 years ago

If I copy the example into a new file and indent the whole text, this is what I get (note that last line):

 f(X) ->
     MyVariable =
         case X of
             true -> ok;
             false -> still_ok
         end,
         MyVariable.

Indenting that last line correctly is very important, because if we don't do that, the indentation of the remaining part of the function will be off. If we indent a whole file, the automatic indentation should be sensible. If it's not 100% the same as Emacs, but consistent with itself, that's still better than trying to be more consistent with Emacs but actually achieving inconsistency even with ourselves.

When I originally wrote the indentation script, I thought about this problem, and I found it quite hard to find out where to "shift back", i.e. where is the end of the expression after the = sign. You even have to consider codes like this:

A =
    B =  % shift
    C,   % no shift
D.       % shift back

A general suggestion for contributions is to test the commit on the test indentation file as described in the README. This not only tests the indentation script with a large example, but it also makes errors visible. (Indentation errors are silent by default.)

I found two errors with executing the test:

  1. When a comment line ends with =, the next line is handled erroneously:

    % ========
       f(X) ->
           ok.
  2. An "echoerr" was left in the code, which makes the test report an error.
evnu commented 9 years ago

Thanks for your reply and your suggestions. I appreciate that you took the time to write that down here. Sorry that I missed to check the commit properly.

I fixed the two errors with an extra commit each. If you'd prefer it, I could also squash them and forcefully push it.

Regarding your more general objections, yes, this is much harder than I thought. I will give this some more thought, but probably your choice of indentation is the reasonable thing to do, considering that indenting has to be fast as well.

hcs42 commented 9 years ago

Thanks for fixing the commits, I appreciate your enthusiasm :)

But to be honest, I wouldn't merge the change until the general issue is solved. I don't want the indentation to lose the property that if the whole code is intended automatically, the result will follow a consistent logic. (Which would not be provided if the indentation were more and more off after each line ending with "=".)

Since this problem is not easy to solve, I suggest closing this PR without merging it, and in case you (or someone else) manages to come up with a consistent solution for this (totally valid) issue, a new PR can be opened. What do you think?

evnu commented 9 years ago

I didn't expect you to merge the fixes, although I probably indicated that with my last message. I just did not want to leave the PR with those stupid errors ;)

As I did not come up with a solution to the general issue, I'll take your advise and close this PR.

Out of curiosity: Have you looked into the erlang emacs mode and how they solve this problem? I planned to do that, but maybe their approach will not work with your code and you already know that.

hcs42 commented 9 years ago

I didn't expect you to merge the fixes, although I probably indicated that with my last message. I just did not want to leave the PR with those stupid errors ;)

OK, I get it now ;)

Out of curiosity: Have you looked into the erlang emacs mode and how they solve this problem? I planned to do that, but maybe their approach will not work with your code and you already know that.

I looked at how Emacs indents Erlang code in general. I don't remember if I had a look at it wrt this specific issue. It may contain ideas that are useful, although the Vim indentation works rather differently from Emacs (Vim parses the Erlang files backwards, while Emacs parses it forwards).

In 2013 I gave a talk about indenting Erlang code (slides are available if you click on the flipchart image), where I showed Emacs indentation mechanism on a high level, although I mostly focused on the Vim indentation (i.e. indent/erlang.vim in this repo). It may help to give some overview.