dhall-lang / dhall-haskell

Maintainable configuration files
https://dhall-lang.org/
BSD 3-Clause "New" or "Revised" License
920 stars 214 forks source link

`dhall format` does not preserve leading separators #1447

Open SiriusStarr opened 5 years ago

SiriusStarr commented 5 years ago

With release 11.0.0 of the standard, leading separators are now allowed (for the sake of nicer diffs). However, dhall format will not preserve such leading separators, e.g.

{
, field1 : Bool
, field2 : Bool
, field3 : Bool
, field4 : Bool
, field5 : Bool
, field6 : Bool
}

is formatted into

{ field1 : Bool
, field2 : Bool
, field3 : Bool
, field4 : Bool
, field5 : Bool
, field6 : Bool
}

which makes it a hassle to actually use leading separators if your workflow includes using dhall format (e.g. using the LSP server which formats on save).

Presumably there should at least be an option to preserve leading separators if they are part of the standard.

sjakobi commented 5 years ago

AFAIK leading separators were introduced especially for formatting record completions (Schema::{…}).

So far the output of dhall format doesn't or at least shouldn't depend on the input formatting. Changing this would IMHO make things quite a lot more complicated, and create a slippery slope for similar requests.

So I'm not a fan of this idea…

SiriusStarr commented 5 years ago

introduced especially for formatting record completions

I see, no such rational was mentioned in the release notes, which only stated:

... which are more commonly used when formatting a multi-line expression so that you can always add/remove entries with only a single-line change.

let example = {
      , x = 1
      , y = True
      }

Since the given example was a simple record, I assumed their use was sanctioned (or even encouraged) in such cases, since it allows for adding/removing items without creating diffs for unchanged lines (as mentioned in the release notes).

So far the output of dhall format doesn't or at least shouldn't depend on the input formatting. Changing this would IMHO make things quite a lot more complicated, and create a slippery slope for similar requests.

A dependence on input formatting is potentially complicated, as you point out. So perhaps leading separators for multi-line comma-separated values should always be inserted? Or a flag to turn it on or off? I just don't see what the point is of having a (in my opinion, useful) formatting feature in the standard if, for all intents and purposes, it can't be used. Sure, you can say "just don't use dhall format" but I feel like that violates the stated design philosophy, which includes things like "Polish" and "Beginner-friendliness". Having to spend a considerable amount of time hand-formatting files doesn't strike me as "everything should 'just work'."

But then I am lazy and perhaps too accustomed to automatic formatting in languages, so YMMV. :woman_shrugging:

Gabriella439 commented 5 years ago

@SiriusStarr: My plan is to eventually format bare record literals with leading commas, too. However, that needs to be preceded by other changes to the formatter, otherwise the result will be unappealing.

Specifically, if I were to introduce leading commas for bare record literals right now, the result would look something like this:

{ example =
    Some
      {
      , foo = 1
      , bar = True
      }
}

What I would like to do is format that like this:

{ example = Some {
    , foo = 1
    , bar = True
    }
}

... but that requires a bit more effort to get that to work. So what I did as a compromise is make it so that only record completions are formatted with leading commas for now.

SiriusStarr commented 5 years ago

@Gabriel439 Excellent, glad to hear that it is planned. Would you like me to close this issue or leave it open to reference when that change is eventually implemented?

Gabriella439 commented 5 years ago

@SiriusStarr: Leave this open, since it's a good reminder and it'll help other people discover this more easily