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

[regression] shrinking function heads to 1-line moving comments between if stanzas #67

Closed
novaugust opened this issue Aug 10, 2023 · 5 comments

Comments

@novaugust
Copy link
Contributor

I have a very large file where a new comment was being moved when nothing else was. The file's so big I'm unsure how to create a minimal repro at this point, but the statements were fairly deeply nested within a function

defmodule ...
  def fun
    if a do
      another_fun = fn ->
        with ...
           # see what i mean? decent bit of nesting

           if foo do
             bar
+
+            # a
+            # long
+            # comment
+            # moved :/
           end

-          # a
-          # long
-          # comment
-          # moved :/
           if widget != wodget do
@novaugust
Copy link
Contributor Author

the function head in question is split over multiple lines, so this is likely a regression in function-head shrinking

@novaugust novaugust changed the title [bug] comment idempotency [regression] shrinking function heads to 1-line moving comments between if stanzas Aug 10, 2023
@novaugust
Copy link
Contributor Author

novaugust commented Sep 16, 2023

i have a theory that this occurs when we try to shrink the function head, but ultimately the formatter disallows the shrink - so it's like

  1. we shrink the function head
  2. we displace the comments an appropriate amount
  3. formatter expands the function head again, as it was too long
  4. however, the comments were still moved up

soooo, a problem resulting from our lazy shrinks? only fix in that case would be to do the heavy lifting of making the document algebra for each head and attempting to shrink it. at that point, might be easier to see if we can modify things upstream in formatter itself to support this collapsing 🤔

then again, if this theory was correct the comments would move up with every run of mix format until they were at the top? which isn't the case. hmm hmm hmm, just need to add a repro test

@APB9785
Copy link

APB9785 commented Nov 7, 2023

Also reproduceable with case:

def my_function(
      very_long_name,
      so_long_that_this_head_will_not_fit_on_one_line
    ) do
  result =
    case foo do
      :bar -> :baz
      :baz -> :bong
    end

  # My comment
  Context.process(result)
end

will become

def my_function(
      very_long_name,
      so_long_that_this_head_will_not_fit_on_one_line
    ) do
  result =
    case foo do
      :bar ->
        :baz

      :baz ->
        :bong

        # My comment
    end

  Context.process(result)
end

@novaugust
Copy link
Contributor Author

wow thanks andrew. for some reason your simple case made me have an epiphany around what's actually going wrong here. i've got a fix coming =)

@novaugust
Copy link
Contributor Author

fixed in 0.10.1

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

No branches or pull requests

2 participants