ndmitchell / hlint

Haskell source code suggestions
Other
1.47k stars 196 forks source link

Fix formatting #1004

Open ndmitchell opened 4 years ago

ndmitchell commented 4 years ago

The formatting of HLint varies significantly between files and within files. There is 2 space indents, 4 space indents. It's just a bit haphazard. We should align on a standard. Ideally with tooling support. My first preference would be fourmolu, for which I am waiting for @parsonsmatt (who was probably being sarcastic anyway...). In the absence of fourmolu, do people have any other preferences?

CC @shayne-fletcher @zliu41 @googleson78 @josephcsible

shayne-fletcher commented 4 years ago

I favor 2 spaces generally.

shayne-fletcher commented 4 years ago

For the avoidance of doubt though, hill die? Me no 😁 Happy to go with the consensus!

parsonsmatt commented 4 years ago

If there's a real need or desire for fourmolu then I can try to get it going. Looks like they made it nice-and-easy by writing indentStep = 2, so it's a one line patch to change it :smile:

ndmitchell commented 4 years ago

As Shayne says, every few people go to die on their formatting hill, but 4 is my preference, and since I also write a fair bit of Rust/Javascript etc where formatters are common and 4 is the answer, it would reduce the mental burden of switching for me. But if all other collaborators come out in favour of 2, then 2 it is, and twormulu can be used.

parsonsmatt commented 4 years ago

ohhhh i hope i don't get flamed for this: fourmolu

josephcsible commented 4 years ago

I have a weak preference for 2 spaces.

EncodePanda commented 4 years ago

stylish-haskell :) [i might be biased :) ]

ndmitchell commented 4 years ago

@EncodePanda an online formatter would let us try before we pick, if something could be made available. The existence for Ormulu is very handy.

EncodePanda commented 4 years ago

@ndmitchell thing about stylish-haskell is that it's not "opinionated" like Ormulu, there is a config file attached to it so you can apply different styles. So the "online formatting" would also have to take that into consideration. I'm happy to provide an example of how formatting could look like, not sure if I have resources to run something as a webserver to try things out.

But I could create a branch of current version of hlint with some default configuration enabled to demo (via diff) what the output would be. Would that make sense?

zliu41 commented 4 years ago

I'm neutral on which formatter to use, or even whether to use a formatter at all vs. just a style guide.

I'd have been a lot happier with Ormolu if it adopted leading ,, -> and => rather than trailing.

I really like this from stylish-haskell README:

This tool tries to help where necessary without getting in the way.

It seems the only downside is that stylish-haskell is based on haskell-src-exts.

btw I also have a weak preference of 2 spaces like some others said.

shayne-fletcher commented 4 years ago

I'd have been a lot happier with Ormolu if it adopted leading ,, -> and => rather than trailing.

You remind me. I ran ormolu over a list literal heavy file a while back and was most displeased with the results (I forget why exactly but I expect it was this).

EncodePanda commented 4 years ago

It seems the only downside is that stylish-haskell is based on haskell-src-exts.

We are moving to ghc-lib-parser over the summer, see https://summer.haskell.org/ideas.html#stylish-haskell-ghc-lib-parser :)

EncodePanda commented 4 years ago

If you decide on:

  1. column width
  2. vertical alignment vs "sane git diffs"

I can create a branch formatted by stylish-haskell for you to decide if this is the right way to go.

shayne-fletcher commented 4 years ago

It seems the only downside is that stylish-haskell is based on haskell-src-exts.

We are moving to ghc-lib-parser over the summer, see https://summer.haskell.org/ideas.html#stylish-haskell-ghc-lib-parser :)

Awesome! 🎉

shayne-fletcher commented 4 years ago

We are moving to ghc-lib-parser over the summer, see https://summer.haskell.org/ideas.html#stylish-haskell-ghc-lib-parser :)

Awesome! 🎉

You may on that note also have an interest in ghc-lib-parser-ex https://github.com/shayne-fletcher/ghc-lib-parser-ex? It's serving stuff to HLint and I'd be more than happy to help pull down things that ~fit~ benefit this project.

EncodePanda commented 4 years ago

You may on that note also have an interest in ghc-lib-parser-ex https://github.com/shayne-fletcher/ghc-lib-parser-ex? It's serving stuff to HLint and I'd be more than happy to help pull down things that ~fit~ benefit this project.

Thanks, will definitely have a look :)

ndmitchell commented 4 years ago

ghc-lib-parser-ex might be useful to the formatter, but even if it's still on haskell-src-exts, I don't think that's a massive problem - we are very familiar with all the issues with it, and how to avoid them, so I don't think it's a problem.

I have no issue with git diffs that are one line longer if the code looks nicer to read. Things which produce O(n) worse diffs are a bit sad, so best avoided.

I also have no sane idea how to suggest we pick between the options. Everyone seems to prefer 2 spaces, apart from me, so I can go with the flow. But which formatter and which flags is not obvious.

shayne-fletcher commented 4 years ago

Pistols at dawn? 😉

ndmitchell commented 4 years ago

🔫

shayne-fletcher commented 4 years ago

Interesting!

Screen Shot 2020-05-23 at 10 30 21 AM

I'm beginning to believe.

shayne-fletcher commented 4 years ago

@felixmulder's most excellent stylish-haskell branch : https://github.com/jaspervdj/stylish-haskell/pull/293

shayne-fletcher commented 4 years ago

@EncodePanda

If you decide on:

  1. column width
  2. vertical alignment vs "sane git diffs"

I can create a branch formatted by stylish-haskell for you to decide if this is the right way to go.

In the absence of other answers, may I suggest:

  1. 78 columns;
  2. I don't know what this means but do advocate git diff minimizations where possible if that helps.
shayne-fletcher commented 4 years ago

@EncodePanda

If you decide on:

  1. column width
  2. vertical alignment vs "sane git diffs"
  1. I don't know what this means but do advocate git diff minimizations where possible if that helps.

Oh! I think I get what you are saying. I am most definitely NOT in favor of vertical alignment (even in the absence of e.g. ImportQualifiedPost).

zliu41 commented 4 years ago

I'm fine with 78 columns. I'm however not sure how enthusiastic Neil is about that, since hlint code regularly exceeds 120 :stuck_out_tongue:

shayne-fletcher commented 4 years ago

I'm fine with 78 columns. I'm however not sure how enthusiastic Neil is about that, since hlint code regularly exceeds 120 😛

🤣

googleson78 commented 4 years ago

I'm also not sure about placing a limit on columns (although obviously I'm not actively contributing :P) - most of the times that I tried to split my pattern matches on ghc's ast into multiple lines it ended up being less readable.. Not sure what the "right" solution is here - maybe more granular functions??

Vertical alignment is great! .. if you're not looking at any diffs ever.

(btw, why 78 instead of 80?)

shayne-fletcher commented 4 years ago

I'm also not sure about placing a limit on columns (although obviously I'm not actively contributing :P) - most of the times that I tried to split my pattern matches on ghc's ast into multiple lines it ended up being less readable.. Not sure what the "right" solution is here - maybe more granular functions??

Vertical alignment is great! .. if you're not looking at any diffs ever.

(btw, why 78 instead of 80?)

actually, yes, you're of course right, 80 cols would be fine. i don't know why my mind jumped to 78... perhaps habit in that it's guaranteed to fit within 80. anyway, with all the real estate offered by modern monitors one might wonder if the sacred 80 column rule is relevant any more. i think it is and feel supported by arguments like these.

ndmitchell commented 4 years ago

I'm more a fan of 120. But I'm not too concerned as long as it doesn't increase the code length dramatically.

EncodePanda commented 4 years ago

I will create PR with code formatted by stylish for you folks to have a look.