elixir-editors / emacs-elixir

Emacs major mode for Elixir
448 stars 94 forks source link

Incorrect indentation of "if" when on the RHS of a pattern match with newline in it #386

Closed whatyouhide closed 7 years ago

whatyouhide commented 7 years ago

When some expressions comes as the right-hand side of a pattern match and it's on the line after the one with the = in it, their indentation is incorrect. Examples are if and for:

# Expected
payload =
  if true do
    %{}
  end

payload =
  for x <- xs, do: {x, 1}

# Current behaviour
payload =
if true do
  %{}
end

payload =
for x <- xs, do: {x, 1}

Note that this does not happen in other cases:

payload =
  case true do
    true ->
      %{}
  end

payload =
  Enum.each(entries, fn(entry) ->
    entry_for_payload
  end)
wpcarro commented 7 years ago

This has been bothering me as well.

Potential fix: Perhaps this is simplistic, but would it be reliable to indent all subsequent lines that immediately follow the = operator until that expression is terminated, @whatyouhide? I can't think of a case right now where this wouldn't work...

Thanks for reporting this.

whatyouhide commented 7 years ago

@wpcarro it sounds like a reasonable thing to do. Off the top of my head, I cannot think of something that comes after = and should not be indented if put on a newline. 🤔

wpcarro commented 7 years ago

@whatyouhide that's good to know. Me neither, and I've spent the past week keeping my eye out for these. Would be interested in hearing feedback from one of the maintainers. I'm in the process of adopting Emacs, so needless to say my Lisp knowledge is paltry otherwise I'd PR something.

mattdeboard commented 7 years ago

@whatyouhide It's not simplistic. Maybe a function like this one to return non-nil if a line ends with =. When that's the case, indentation level could be incremented.

mattdeboard commented 7 years ago

To expand on that, you could change lines 479-480 like so:

      ((or (elixir-smie-last-line-end-with-block-operator-p)
           (elixir-smie-last-line-end-with-equal-p)) ; this is a made-up function name
       (smie-rule-parent elixir-smie-indent-basic))

Alternatively you could refactor elixir-smie-last-line-end-with-block-operator-p to account for -> or =. Though I suspect, but don't know, it would be useful to have them separate.

mattdeboard commented 7 years ago

Now that I think about it actually, the reason you see it with for and if but not with e.g. case is because probably SMIE sees case and there's logic to indent if the line begins with that, or something.

Ugh

mattdeboard commented 7 years ago

Maybe related to #376?

whatyouhide commented 7 years ago

Closing for inactivity and in favor of the formatters that are being developed which will take care of indentation.

dgutov commented 5 years ago

I could look into it. We have this behavior "right" in ruby-mode.