Friendly ping for @georgefst. Would you mind doing an initial review when you have some time? 🙂

Fixed the YAML config file parsing for my new option. AFAIK, the only remaining work (besides comments from review) is to add another test case in a 3rd commit.

When writing some test cases I ended up finding a bug in the implementation. Apparently, it can't properly break function arguments when they're over 80 characters long.

Here are some examples of what works and what doesn't work (i.e. I get an error because the AST changed after formatting).

-- ------------------------ These work ------------------------
-- ---------------------------------------------------------------80 characters|

longFunction0 :: String -> String -> String -> Maybe Int -> Maybe Int -> Maybe Int -> String -> [String]
longFunction0 veryLongArg1 a b c d e f = ["a list", "of strings", "that will break the", "column limit"]

longFunction1 :: String -> String -> String -> Maybe Int -> Maybe Int -> Maybe Int -> String
longFunction1 
    veryLongArg1 veryLongArg2 veryLongArg3 veryLongArg4 veryLongArgument5 veryLongArg6 = undefined

longFunction12 veryLongArg1 veryLongArg2 veryLongArg3 veryLongArg4 veryLongArgument5 
longFunction12 :: String -> String -> String -> Maybe Int -> Maybe Int -> Maybe Int -> String -> String
    veryLongArg6 veryLongArg6 = undefined


-- If the arguments stop right at 80 characters, it works.
longFunction2 :: String -> String -> String -> Maybe Int -> Maybe Int -> Maybe Int -> String
longFunction2 veryLongArg1 veryLongArg2 veryLongArg3 veryLongArg4 a veryLongArg6 =
    let aVeryLongLine = ["list one", "list one", "list one", "list one", "list one", "list one"]
    in undefined

-- ------------------------ Below here doesn't work ------------------------

longFunction3 :: String -> String -> String -> Maybe Int -> Maybe Int -> Maybe Int -> String
longFunction3 veryLongArg1 veryLongArg2 veryLongArg3 veryLongArg4 veryLongArgument5 veryLongArg6 = undefined

I formatted this using the -u flag to see the output and the last example (the one that breaks) becomes this:

longFunction3 ::
    String -> String -> String -> Maybe Int -> Maybe Int -> Maybe Int -> String
longFunction3
veryLongArg1
veryLongArg2
veryLongArg3
veryLongArg4
veryLongArgument5
veryLongArg6 = undefined

I think that if the user doesn't break the arguments themselves, fourmolu can't distinguish the function name from the arguments... My only idea to make this work would be to disable this check for this specific case (function arguments). That would probably involve adding an argument to switchLayout, which calls spansLayout, to not use the column limit. The problem is that I don't have enough context on the code to find out where these functions will be called when parsing function definitions...

@expipiplus @parsonsmatt Could you maybe help me with that? Any code pointers that I could go over to disable the column limit in the switchLayout calls for function arguments?

I was actually able to find the place that I think switchLayout is used for the arguments list in the function definition. I just put up a 3rd commit with a fix that solved the issue.

I actually found a bug in this implementation that breaks idempotence in some cases... Since idempotence is an important property for a formatter, I don't want to ship this without a fix...
It seems to be related to the indentation of blocks after do statements in long lines (see examples at the bottom).

@georgefst, in one of your comments in #6 you said that "idempotence is a rather fragile thing". Were there any other features/improvements in the past that broke idempotence? If so, how were they fixed? I would appreciate anything that could help me debug and fix this issue.

Thanks!

Original code:

testFund :: Maybe Int
testFund = 
  firstTest oneFunctionArgument abcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcde $ do
    result <- aRandomResult

After formatting with column limit set to 80:

testFund :: Maybe Int
testFund =
    firstTest
        oneFunctionArgument
        abcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcde $ do
        result <- aRandomResult

After formatting the output again:

testFund :: Maybe Int
testFund =
    firstTest
        oneFunctionArgument
        abcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcdeabcde
        $ do
            result <- aRandomResult

Of course, formatting this snippet with the -c flag to check idempotence fails.

I tested fourmolu with this option in thousands of files and found out that this isn't a big idempotence issue because it has a stopping point. i.e. the formatter won't undo something it did and get into a vicious cycle of doing/undoing (e.g. adding/removing a line break).

Also, this feature doesn't introduce any major issue that breaks the files (e.g. by changing their AST).

Considering what I've listed above and the fact that idempotence seems to be a very fragile property (IIRC, initial versions of Ormolu were not idempotent and it was still released and used with this known limitation), I was wondering if we could consider releasing this feature as long as it's clear that it has the limitation of affecting idempotence.

@kukimik, @brandonchinn178, I've seen you engage in other issues/PRs so I'm wondering if you have any thoughts about this? Any feedback would be very welcome! 🙂

I'm alright with the idempotence issue seeing as:

• we're fairly sure it doesn't get in to a loop
• the documentation of --column-limit makes the issue clear (maybe it'd be nice to categorise the kind of inputs that cause issues, or give examples?)
• the option is off by default

Are you saying its not idempotent? As in, running fourmolu multiple times could iteratively format line breaks? Can you give an example?

In general, though, I'm generally in favor of this. Specifically with regards to breaking import statements.

Thank you @georgefst @brandonchinn178 for your responses and feedback!

@georgefst, I'm happy to hear that! I just pushed one more commit where I add an example in the test files, update the test runner and add a line to the README making the column-limit limitation clearer. WDYT?

@brandonchinn178 yes, in some cases the formatter will not be idempotent. I just added an example here. In practice though, it doesn't look like the file will change when formatted for a 3rd time and the changes between the first and second formatting are very minor (e.g. indentation of statements nested inside a do or a $).

@georgefst LMK if there's anything else I need to add/change to merge this!
I'm very excited to ship this, so people that hate long lines (like me) don't have to worry about breaking them manually anymore 😬

Is this ready to merge @brandonchinn178 @georgefst ?

Hey @georgefst! I noticed that there has been some significant changes committed in the past week, which is great!

I already rebased this PR and tested everything, so I'm wondering if we could get this merged?

Hi. What is needed for getting this merged? I want it :)

👋 @gustavoavena
Thank you for opening this PR and being patient while we sorted out how to handle all the new contributions for new configuration options!

If you haven't noticed, we finally merged in a new testing framework for configuration options (#169)! This means we can finally address the backlog of open PRs.

Please do the following steps

• Rebase against main

  git remote add upstream git@github.com:fourmolu/fourmolu
  git fetch upstream
  git rebase -i upstream/main

• Resolve merge conflicts

• Ensure a file has been added to changelog.d/ (see changelog.d/README.md)

• Ensure configuration docs in README.md have been updated

• Ensure fourmolu.yaml updated to stay in sync with config in README.md

• Ensure tests have been added

You can use #135 as a reference for this checklist.

@brandonchinn178 Thank you for the update and for all the you've been putting in recently. Unfortunately I haven't had the bandwidth to get back to this, but once I have some more time, I'll work on rebasing and writing test to ship this :)

Gentle ping - will work on getting this PR merged resume?

@arybczak no ones currently working on this PR. feel free to branch off and start a new PR

I took up on the offer and branched off on this work in the attempt to revive the feature in #305. Thank you for your work, @gustavoavena!

This work has been merged via #305!