Elixir: All comments within pipe expressions are shifted to top

[seantanly]

Environment

• Elixir & Erlang versions (elixir --version): Elixir 1.6.0, Erlang OTP 20.2.2
• Operating system: MacOS

Current behavior

Before mix format

data
|> func1() # comments1
# comments2
|> func2()

After mix format

# comments1
# comments2
data
|> func1()
|> func2()

Expected behavior

data
# comments1
|> func1()
# comments2
|> func2()

[josevalim]

Can you please provide actual code samples to help move the discussion forward? Thank you.

[seantanly]

I assume the discussion is going towards whether we should even use comments within pipeline statements.

On looking at my codebases, I realize how rare I use comments within pipe statements. I understand complex/ambiguous code blocks should be extracted to well-named functions with comments when necessary.

My use case is making use of IO.inspect within pipe statements for quick print/debug cycle. In doing so, I cannot do format-on-save (mix format) since that will throw the commented IO.inspect() statements out of position.

Looking back, my knee-jerk reaction happened because the existing line comments within pipe statements is shifted out of context without warning, and the comments effectively became wrong and misleading. Going forward, while "comments in pipe statements" is not supported, perhaps mix format should emit a warning per such comment and remove them in the final output. This should set the right expectation for the developer.

[josevalim]

I assume the discussion is going towards whether we should even use comments within pipeline statements.

Partially yes. The tricky part is that we need to explicitly support interleaved comments in the desired constructs. So we not only need to discuss if we want to support interleaved comments in pipelines, we also need to discuss if we want to generalize it to other operators or not.

But in general I would say supporting it for pipelines would be fairly straight-forward because the operator always starts on the current line. So I am inclined to do it once we have a full discussion on pros and cons. :)

[AndrewDryga]

I can't recover where is the exact code, but I had a comment that was like a "TODO: replace this with blah blah blah". And it lost it's sense after it was moved to the top of pipeline.

[chrismcg]

I have often commented out parts of pipelines when exploring ideas during development. This moving comments behaviour combined with a format on save option in an editor wouldn't work together very well in this scenario.

[josevalim]

@chrismcg that's a very good example. Let's tackle this then.

[josevalim]

Thanks everyone, I have fixed this in master.

I have also used this opportunity to support comments in the middle of when and |. I actually find the final code is clearer than before, while adding a feature, which is always nice. :)

However, the changes were a bit larger than I expected and it requires a bit more testing, so I won't backport it to v1.6 branch for now.

[chrismcg]

I see this didn't make it into 1.6.2. Is it going to be back ported?

[josevalim]

@chrismcg I have backported it to v1.6 now. By the time v1.6.3 is out we will have tested it enough to include it in a minor release.