Skip to content

Fixes for function arrows#247

Merged
brandonchinn178 merged 17 commits intomainfrom
function-arrows
Nov 8, 2022
Merged

Fixes for function arrows#247
brandonchinn178 merged 17 commits intomainfrom
function-arrows

Conversation

@brandonchinn178
Copy link
Copy Markdown
Collaborator

Found some edge cases for function arrows, and was able to simplify the code without the PrevTypeCtx machinery.

Some of the changes I made seem to still pass tests (e.g. moving things in/out of a located block). If we find a regression or issue later, we can fix it later with an added test

@github-actions
Copy link
Copy Markdown

github-actions bot commented Nov 1, 2022

👋 @brandonchinn178
Thank you for raising your pull request.
Please make sure you have followed our contributing guidelines in DEVELOPER.md. We will review it as soon as possible!

Reviewer: Please verify the following things have been done, if applicable.

  • A file has been added to changelog.d/
  • "Configuration > Available options" section in README.md has been updated
  • "Configuration > Specifying configuration" section in README.md has been updated
  • fourmolu.yaml updated to stay in sync with config in README.md
  • Tests have been added

@brandonchinn178
Copy link
Copy Markdown
Collaborator Author

cc @3kyro

@3kyro
Copy link
Copy Markdown
Contributor

3kyro commented Nov 5, 2022

@brandonchinn178 There seems to be a regression on multi line arguments with interleaved comments. You can test the following:

withComments
  -- Comment
  :: Int
  -- Comment
  -> Int
  -- Comment
  -> Int

which gets formatted as:

withComments
    :: -- Comment
    Int
    -> -- Comment
    Int
    -> -- Comment
    Int

@brandonchinn178
Copy link
Copy Markdown
Collaborator Author

@3kyro thanks! The situation isn't much better on main, so it's not too much of a regression

foo
    :: -- asdf
    Int
    -- bar
    -> Bool

But I'll take a look!

@3kyro
Copy link
Copy Markdown
Contributor

3kyro commented Nov 5, 2022

The situation isn't much better on main

Indeed it's not 😞

Apart from this, the fixes look like a big improvement and I didn't notice any other formatting change regarding leading function arrows.

The "::" for SPECIALIZE should be aligned with the commas, not the
function arrows. Technically speaking, we should respect 'comma-style'
here, but that can be done separately. For now, just revert this code to
before the function-arrows work.
@brandonchinn178
Copy link
Copy Markdown
Collaborator Author

@3kyro just pushed 3 new commits! should have the comments fixed, and i also found an issue with leading arrows + explicit type signatures for expressions

@brandonchinn178 brandonchinn178 merged commit 7334f6d into main Nov 8, 2022
@brandonchinn178 brandonchinn178 deleted the function-arrows branch November 8, 2022 00:41
michaelpj added a commit to michaelpj/fourmolu that referenced this pull request Oct 6, 2024
This adds some simple sorting of constraints. There are a few potential
deficiencies:

1. It doesn't sort nested constraint tuples. We could solve this by
   carrying around some information about whether we are in a constraint
   context in `p_hsType`, but that seeemed moderately disruptive.
2. It doesn't handle constraint tuple synonyms. I think this is
   unavoidable since we can't know they're constraint tuples without type
   information.
3. The sorting is very crude, just sorting on the `Outputable`
   representation of  the type. I think this will more-or-less doe what
   people expect, but perhaps it's too expensive?

Fixes fourmolu#247
michaelpj added a commit to michaelpj/fourmolu that referenced this pull request Oct 6, 2024
This adds some simple sorting of constraints. There are a few potential
deficiencies:

1. It doesn't sort nested constraint tuples. We could solve this by
   carrying around some information about whether we are in a constraint
   context in `p_hsType`, but that seeemed moderately disruptive.
2. It doesn't handle constraint tuple synonyms. I think this is
   unavoidable since we can't know they're constraint tuples without type
   information.
3. The sorting is very crude, just sorting on the `Outputable`
   representation of  the type. I think this will more-or-less doe what
   people expect, but perhaps it's too expensive?

Fixes fourmolu#247
michaelpj added a commit to michaelpj/fourmolu that referenced this pull request Oct 6, 2024
This adds some simple sorting of constraints. There are a few potential
deficiencies:

1. It doesn't sort nested constraint tuples. We could solve this by
   carrying around some information about whether we are in a constraint
   context in `p_hsType`, but that seeemed moderately disruptive.
2. It doesn't handle constraint tuple synonyms. I think this is
   unavoidable since we can't know they're constraint tuples without type
   information.
3. The sorting is very crude, just sorting on the `Outputable`
   representation of  the type. I think this will more-or-less doe what
   people expect, but perhaps it's too expensive?

Fixes fourmolu#247
michaelpj added a commit to michaelpj/fourmolu that referenced this pull request Oct 6, 2024
This adds some simple sorting of constraints. There are a few potential
deficiencies:

1. It doesn't sort nested constraint tuples. We could solve this by
   carrying around some information about whether we are in a constraint
   context in `p_hsType`, but that seeemed moderately disruptive.
2. It doesn't handle constraint tuple synonyms. I think this is
   unavoidable since we can't know they're constraint tuples without type
   information.
3. The sorting is very crude, just sorting on the `Outputable`
   representation of  the type. I think this will more-or-less doe what
   people expect, but perhaps it's too expensive?

Fixes fourmolu#247
brandonchinn178 pushed a commit that referenced this pull request Oct 7, 2024
This adds some simple sorting of constraints. There are a few potential
deficiencies:

1. It doesn't sort nested constraint tuples. We could solve this by
   carrying around some information about whether we are in a constraint
   context in `p_hsType`, but that seeemed moderately disruptive.
2. It doesn't handle constraint tuple synonyms. I think this is
   unavoidable since we can't know they're constraint tuples without type
   information.
3. The sorting is very crude, just sorting on the `Outputable`
   representation of  the type. I think this will more-or-less doe what
   people expect, but perhaps it's too expensive?

Fixes #247
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.

2 participants