Skip to content
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

Add correct indentation for 'with' keyword #350

Merged
merged 2 commits into from
Apr 28, 2016
Merged

Add correct indentation for 'with' keyword #350

merged 2 commits into from
Apr 28, 2016

Conversation

tonini
Copy link
Contributor

@tonini tonini commented Apr 27, 2016

related to #349

Examples

defmodule Foo do

  def bar do
    with {:ok, width} <- Map.fetch(opts, :width),
         {:ok, height} <- Map.fetch(opts, :height) do
      {:ok, width * height}
    else
      :error ->
        {:error, :wrong_data}
    end
  end

end
defmodule Foo do
  def bar do
    with {:ok, width} <- Map.fetch(opts, :width),
         {:ok, height} <- Map.fetch(opts, :height),
      do: {:ok, width * height}
  end
end
defmodule Foo do
  def bar do
    with({:ok, width} <- Map.fetch(opts, :width),
         {:ok, height} <- Map.fetch(opts, :height),
      do: {:ok, width * height})
  end
end

This PR will also fixes #351

@tonini
Copy link
Contributor Author

tonini commented Apr 27, 2016

/cc @gausby @whatyouhide

@whatyouhide
Copy link

As I said in #349, I don't think with() would be used (I never used for() myself) and the indentation it has now it's confusing anyways IMO (why are the clauses not aligned, but just when there are parens?).

If we want to support with(), then I'd say we shouldn't do anything different than regular with, i.e., aligning clauses with each other and do:/else: one level more than with.

@tonini
Copy link
Contributor Author

tonini commented Apr 27, 2016

@whatyouhide @gausby ok I updated the examples how it works now.

@gausby
Copy link
Contributor

gausby commented Apr 27, 2016

@whatyouhide I use with/1 with parenthesis to get around some indentation issues, so if we keep it as is for the with() case I would be a happy camper; and I would start using the “normal”-style when this PR is merged.

@tonini
Copy link
Contributor Author

tonini commented Apr 28, 2016

As @whatyouhide already mentioned, the behavior of with/1 should be the same as without. (Sorry @gausby ;-)

I think also this way because it's hard to support different behaviors at all in cases of with parentheses and without.

I would like to merge this PR now, if you're guys ok with it. 👍

@whatyouhide
Copy link

I'm 👍 for it! :)

@tonini tonini merged commit e2b8578 into master Apr 28, 2016
@tonini tonini deleted the with-statemnts branch April 28, 2016 08:19
@gausby
Copy link
Contributor

gausby commented Apr 28, 2016

@tonini but I'll just switch my coding style, I only did the ()'s to work around the indentation issue that this pr solves ^_^

(I am lazy like that)

Thanks for fixing this!

J3RN pushed a commit to J3RN/emacs-elixir that referenced this pull request Apr 24, 2021
Signed-off-by: Drew Olson <drew.olson@greensill.com>

Co-authored-by: dix <william.dix@greensill.com>
J3RN pushed a commit to J3RN/emacs-elixir that referenced this pull request Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong indentation with tuples in "case" statements
3 participants