Skip to content

Use import export comma option in commaDel'#187

Merged
brandonchinn178 merged 2 commits intofourmolu:mainfrom
3kyro:174-fix-ie-inconsistent-formatting
May 30, 2022
Merged

Use import export comma option in commaDel'#187
brandonchinn178 merged 2 commits intofourmolu:mainfrom
3kyro:174-fix-ie-inconsistent-formatting

Conversation

@3kyro
Copy link
Copy Markdown
Contributor

@3kyro 3kyro commented May 21, 2022

commaDel' now checks the import export lists comma option. This makes nested list imports being formatted similarly to regular lists.
fixes #174
cc @brandonchinn178

@brandonchinn178
Copy link
Copy Markdown
Collaborator

Thanks! I'm on vacation right now, so I'll review + merge when I get back

Copy link
Copy Markdown
Collaborator

@brandonchinn178 brandonchinn178 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! Left a comment; feel free to incorporate it or resolve it as "wont do", and I'll merge after.

Now to figure out why CI isn't running on here...

Comment thread src/Ormolu/Printer/Meat/ImportExport.hs Outdated
Comment on lines +183 to +186
commaDel' =
getPrinterOpt poImportExportCommaStyle >>= \case
Leading -> breakpoint' >> comma >> space
Trailing -> comma >> breakpoint
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to unify this with the version in Ormolu.Printer.Combinators. What do you think about moving commaDel' into Ormolu.Printer.Combinators and defining

commaDel = commaDel' poCommaStyle
commaDelImportExport = commaDel' poImportExportCommaStyle

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've added commaDelImportExport as you suggest.

@brandonchinn178
Copy link
Copy Markdown
Collaborator

Ok sorry, just fixed CI on main; can you rebase + push?

@3kyro 3kyro requested a review from brandonchinn178 May 30, 2022 19:49
@brandonchinn178 brandonchinn178 merged commit c16bfd2 into fourmolu:main May 30, 2022
@brandonchinn178
Copy link
Copy Markdown
Collaborator

Awesome, thanks!!

brandonchinn178 added a commit that referenced this pull request May 30, 2022
brandonchinn178 added a commit that referenced this pull request May 30, 2022
@3kyro
Copy link
Copy Markdown
Contributor Author

3kyro commented May 30, 2022

I've just realized I misplaced commaDelImportExport export position. I'll create a small PR for this.

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.

Inconsistent formatting with import-export-comma-style: leading

2 participants