tweag / ormolu

A formatter for Haskell source code
https://ormolu-live.tweag.io
Other
958 stars 83 forks source link

Minimise diff in module header #409

Closed steshaw closed 3 years ago

steshaw commented 5 years ago

Hi, I was just reading the first release announcement. What do you think about changing:

module Ormolu
  ( ormolu,
    ormoluFile,
    ormoluStdin,
    -- ...
    withPrettyOrmoluExceptions,
  )
where

to:

module Ormolu (
  ormolu,
  ormoluFile,
  ormoluStdin,
  -- ...
  withPrettyOrmoluExceptions,
) where

It would help to minimie diffs when adding an export that would appear before ormolu, say normalise. I also like how the indentation is minimised — now the exports have just one level of indentation rather than two.

diff --git a/Ormolu.hs b/Ormolu.hs
index b039aab..fddbdc1 100644
--- a/Ormolu.hs
+++ b/Ormolu.hs
@@ -1,5 +1,6 @@
 module Ormolu
-  ( ormolu,
+  ( normalise,
+    ormolu,
     ormoluFile,
     ormoluStdin,
     -- ...

compared with:

diff --git a/Ormolu.hs b/Ormolu.hs
index d263f20..84b386c 100644
--- a/Ormolu.hs
+++ b/Ormolu.hs
@@ -1,4 +1,5 @@
 module Ormolu (
+  normalise,
   ormolu,
   ormoluFile,
   ormoluStdin,
mrkkrp commented 5 years ago

It does minimize the diffs but also makes formatting of parens inconsistent. We won't be able to even use the normal parens combinator here, we'll need something custom. One could argue that we should format parens everywhere like this, but I do not think it's a good idea.

curiousleo commented 5 years ago

One could argue that we should format parens everywhere like this

I think that is worth considering. Having just one level of indentation rather than two could be beneficial not just in module imports, but everywhere.

mboes commented 5 years ago

@curiousleo True. But do you also mean in expressions? How would that work? Specifically, how would you like the following input to be formatted?

foo bar baz (qux,
                    quux)
steshaw commented 4 years ago

A way to view my suggestion is: module imports are not expressions, It would be nice to keep the principle of minimising diffs for module imports.

I acknowledge that it would require some programming as it's not the same as the parens combinator for expressions but it's worth it — I often do this manually today :disappointed:. If you agree, I could help with the programming if you point me in the right direction :smile:.

georgefst commented 4 years ago

AFAICT, the only downside here is that it slightly complicates the implementation.

Would you accept a PR?

georgefst commented 4 years ago

To be clear, I'm not talking about expressions. For that we'd probably have to wait on some form of ExtraCommas...

mrkkrp commented 4 years ago

I already expressed my opinion. I think it is not something we should change.

mitchellwrosen commented 4 years ago

+1 to this idea, I think minimizing diffs is more important than being consistent here.

Sounds like @georgefst is willing to put up a patch - is this issue left open to see how many people come along to voice an opinion?

georgefst commented 4 years ago

I think minimizing diffs is more important than being consistent here.

I'd go so far as to say that the status quo is also only really 'consistency' between two fundamentally different things that happen to have somewhat similar syntax.

mrkkrp commented 4 years ago

I'd go so far as to say that the status quo is also only really 'consistency' between two fundamentally different things that happen to have somewhat similar syntax.

That's about right, but there is some value in purely visual consistency between different constructions even when they lack semantic connections. This serves the vague purpose of aesthetic pleasure, of which many tech people are thoroughly deprived. Should there be something consistent about the visual representation of source code? Perhaps the consistency helps the human brain to read it? Difficult questions, questions that still await their researcher. Nevertheless, I believe that those questions should not be dismissed lightly in favor of trite longing for minimal diffs no matter how alluring even smallest gains of this sort may appear.


is this issue left open to see how many people come along to voice an opinion?

Not really. We do not know the number of people who are against this change, and those may be just silent users who do not even suspect about this issue.

steshaw commented 4 years ago

From an aesthetic point of view, there are two considerations

Currently, Ormolu would generate the following:

module Hi
  ( hello,
    world,
  )
where

I prefer the following for aesthetic reasons as well as minimising diffs:

module Hi (
  hello,
  world,
) where

With Ormolu, there is 2 space indentation. However, with module exports, there are 4 (i.e. the hello is indented by 4 spaces from the beginning of the line). With the alternative rendering, it is 2.

For vertical spacing, the principle I follow is to maximise the amount of code on the screen as long as other considerations are not violated. In this case, we have 5 lines in the original rendering and 4 with the alternative rendering. It's a small difference but important to me aesthetically.

georgefst commented 4 years ago

For me, the suggested approach wins the aesthetic argument for it's internal consistency (i.e. line-by-line). But I appreciate that's subjective.

mrkkrp commented 3 years ago

I think we can close this one. My (I understand, rather annoying) opinion has not changed and the feature has since been implemented in fourmolu.

mitchellwrosen commented 3 years ago

Sounds good!

Just thinking out loud here, but with a renewed interest in the Haskell language and ecosystem as a whole (Haskell Foundation) I think it is only a matter of time before we collectively decide, "ok we only need one formatter" (see gofmt, rustfmt, prettier, etc).

I therefore worry a bit when tickets are closed with the rationale that it's implemented in a fork. Perhaps we could brainstorm ways to work towards uniting these tools eventually.

mrkkrp commented 3 years ago

I don't think that it is certain that one formatter will be selected as "the only solution."

One of the main design principles behind Ormolu is that it has one unconfigurable style. While this principle gets criticized now and then, especially when people do not like the style that has been chosen, I think de facto it has been working great. It allowed us to make the most functional formatter for Haskell so far.

So if we want to continue benefit from this design, there should be something that prevents random fluctuations in style as well as the temptation of forsaking the principle itself. Sometimes a proposed change is obviously good, oftentimes it is not. Sometimes it is just a matter of taste and is difficult to judge. This is where I have to vote according to my preferences as the current maintainer and the original creator of the tool.

I can't help it, but formatting parens like this in this single instance while they are formatted differently everywhere else just makes me cringe. It just doesn't look good or consistent. I think there is value in consistency and aesthetics. I'd be fine with this change if we also managed to change formatting other uses of parens and possibly other paired punctuation, but frankly it seems like going into a lot of trouble for very little benefit.

mitchellwrosen commented 3 years ago

I totally agree with the spirit of having one unconfigurable format, and if it has also made the implementation simpler, that's a win, too :)

I guess, to clarify my concern, contrary to my original comment I do not feel it is inevitable that this community will push for a standard formatter, however I feel it would be (significantly) better if there was one.

In golang, for example, that's gofmt, which admits no configuration. In javascript, that's prettier, which does.

I would love to see ormolu become that de facto formatter, since it has the most solid implementation and test suite coverage by far, and that's awesome. And perhaps it is already on its way to becoming the standard, but the existence of fourmolu is certainly mudding the waters a bit, especially when features are closed here with the justification that if you really want this, you can use fourmolu instead (which is true!).

Perhaps what would help elevate the status of ormolu is more of a sense of community ownership and input. I've seen many issues that were closed because of stylistic/asthaetic differences between you (the maintainer) and the issue creater. This is completely understandable and justified for an open-source project, and I believe you when you say it has improved the quality of the codebase, too.

But I do wonder if, for a tool so important to the health of a language ecosystem overall (in addition to a language server, a debugger, an so on), it may make sense to leave issues more open to debate by the broader community, perhaps advertised on reddit/twitter/discourse, or something, so that we can, as a community, feel like ormolu has made a serious effort to take everyone's input before coming down on one side of the fence or the other :)

In a distant future I can imagine one of the many competing formatters being blessed and moved to some more official location like haskell/hsfmt on GitHub, with its own little RFC process, and so on. That seems like an awesome place to be for the community and ecosystem.