Dhall-haskell: Dhall format/lint pretty printer improvements
The pretty printer has (IMO) a few issues, currently my workflow is using dhall lint and then manually fix these.
For example taking this freshly formatted file:
LocalVolume.dhall
let types = ../types.dhall
let defaults = ../defaults.dhall
let settings = ../settings.dhall
let utils = ../utils.dhall
in λ(v : ./LocalVolume.dhall)
→ defaults.PersistentVolume
⫽ { metadata =
defaults.ObjectMeta
⫽ { name = v.name, namespace = Some v.namespace }
, spec =
Some
( defaults.PersistentVolumeSpec
⫽ { capacity =
[ { mapKey = "storage", mapValue = v.size } ]
, accessModes =
[ "ReadWriteOnce" ]
, persistentVolumeReclaimPolicy =
Some "Retain"
, storageClassName =
Some "local-storage"
, local =
Some { path = "/data/${v.directory}", fsType = None Text }
, nodeAffinity =
{ required =
Some
{ nodeSelectorTerms =
[ defaults.NodeSelectorTerm
⫽ { matchExpressions =
[ { key =
"kubernetes.io/hostnames"
, operator =
"In"
, values =
utils.NonEmpty.toList
Text
settings.hosts
}
]
}
]
}
}
}
)
}
: types.PersistentVolume
I have to transform it to land at this:
LocalVolume.dhall
let types = ../types.dhall
let defaults = ../defaults.dhall
let settings = ../settings.dhall
let utils = ../utils.dhall
in λ(v : ./LocalVolume.dhall)
→ defaults.PersistentVolume
⫽ { metadata =
defaults.ObjectMeta
⫽ { name = v.name, namespace = Some v.namespace }
, spec =
Some
( defaults.PersistentVolumeSpec
⫽ { capacity = [ { mapKey = "storage", mapValue = v.size } ]
, accessModes = [ "ReadWriteOnce" ]
, persistentVolumeReclaimPolicy = Some "Retain"
, storageClassName = Some "local-storage"
, local = Some { path = "/data/${v.directory}", fsType = None Text }
, nodeAffinity =
{ required =
Some
{ nodeSelectorTerms =
[ defaults.NodeSelectorTerm
⫽ { matchExpressions =
[ { key = "kubernetes.io/hostnames"
, operator = "In"
, values =
utils.NonEmpty.toList
Text
settings.hosts
}
]
}
]
}
}
}
)
}
: types.PersistentVolume
As you can see there are basically 3 changes I do:
- Do not put an empty line between single line
lets - Do not wrap record values if less than 80 characters
- (Not visible above because happend to be right already) Align the arrow or the record merge operator 4 characters from the left, not the type signature
I just want to open this to have a discussion about the mentioned changes, I won't have time to work on these any time soon
jvanbruegge
All 11 comments
This file is probably a better example as it uses all three rules
vault.dhall
let defaults = ../defaults.dhall
let types = ../types.dhall
let apiDefaults = ../apiDefaults.dhall
let apiFunctions = ../apiFunctions.dhall
let vaultContainer =
defaults.Container
⫽ { name =
"vault"
, image =
Some "registry.hub.docker.com/library/vault:1.2.0-beta2"
, command =
[ "sh" ]
, args =
[ "/scripts/runVault.sh" ]
, env =
[ { name =
"VAULT_LOCAL_CONFIG"
, value =
Some ./vault.tcl as Text
, valueFrom =
defaults.EnvVarSource
}
]
, securityContext =
Some
( defaults.SecurityContext
⫽ { capabilities =
Some { drop = [] : List Text, add = [ "IPC_LOCK" ] }
}
)
, ports =
[ defaults.ContainerPort
⫽ { containerPort =
8200
, name =
Some "vault-port"
, protocol =
Some "TCP"
}
]
, volumeMounts =
[ defaults.VolumeMount
⫽ { mountPath = "/vault", name = "vault-data" }
, defaults.VolumeMount
⫽ { mountPath = "/scripts", name = "vault-config" }
]
}
: types.Container
let config =
apiDefaults.SimpleDeployment
⫽ { name =
"vault"
, namespace =
"vault"
, containers =
[ vaultContainer ]
, volumes =
[ defaults.Volume
⫽ { name =
"vault-data"
, persistentVolumeClaim =
Some { claimName = "vault-claim", readOnly = None Bool }
}
, defaults.Volume
⫽ { name =
"vault-config"
, configMap =
Some
( defaults.ConfigMapVolumeSource
⫽ { name = Some "vault-config" }
)
}
]
, serviceAccountName =
Some "vault-auth"
}
in apiFunctions.mkStatefulSet config
After manual transformation:
vault.dhall
let defaults = ../defaults.dhall
let types = ../types.dhall
let apiDefaults = ../apiDefaults.dhall
let apiFunctions = ../apiFunctions.dhall
let vaultContainer =
defaults.Container
⫽ { name = "vault"
, image = Some "registry.hub.docker.com/library/vault:1.2.0-beta2"
, command = [ "sh" ]
, args = [ "/scripts/runVault.sh" ]
, env =
[ { name = "VAULT_LOCAL_CONFIG"
, value = Some ./vault.tcl as Text
, valueFrom = defaults.EnvVarSource
}
]
, securityContext =
Some
( defaults.SecurityContext
⫽ { capabilities = Some { drop = [] : List Text, add = [ "IPC_LOCK" ] }
}
)
, ports =
[ defaults.ContainerPort
⫽ { containerPort = 8200
, name = Some "vault-port"
, protocol = Some "TCP"
}
]
, volumeMounts =
[ defaults.VolumeMount
⫽ { mountPath = "/vault", name = "vault-data" }
, defaults.VolumeMount
⫽ { mountPath = "/scripts", name = "vault-config" }
]
}
: types.Container
let config =
apiDefaults.SimpleDeployment
⫽ { name = "vault"
, namespace = "vault"
, containers = [ vaultContainer ]
, volumes =
[ defaults.Volume
⫽ { name = "vault-data"
, persistentVolumeClaim =
Some { claimName = "vault-claim", readOnly = None Bool }
}
, defaults.Volume
⫽ { name = "vault-config"
, configMap =
Some
( defaults.ConfigMapVolumeSource
⫽ { name = Some "vault-config" }
)
}
]
, serviceAccountName = Some "vault-auth"
}
in apiFunctions.mkStatefulSet config
@jvanbruegge: For the single-line lets I think that's probably fine, but I'd like to hear from more people before making the change.
I would also like to add another suggestion: maybe apply the same blank line rules for record fields and for let bindings. dhall format is a little bit inconsistent here because it never inserts blank lines for record fields and always inserts blank lines for let definitions, even though they are both essentially the same thing (a record is just another way to bind multiple values).
For single-line records, the formatter will already format them on one line if they don't pass column 80. However, what I could do is make the maximum column parametrizable instead of hardcoding it to 80 characters.
For the type signature, I might suggest an intermediate solution, which is that a multi-line expression of the form x: t (assuming x is large) could be formatted like this:
x
: t
... instead of the current formatting, which is:
x
: t
That won't dedent it as much as you requested, because I would still like to retain a two-space indent in between the variable bound by a let definition and its right hand side:
let x =
e
in …
Gabriel439
on 13 Jul 2019
Also, I will migrate this issue to the dhall-haskell project since it's specific to the Haskell implementation
Gabriel439
on 13 Jul 2019
That won't dedent it as much as you requested, because I would still like to retain a two-space indent in between the variable bound by a let definition and its right hand side:
What I dislike about this is that the indent depends on the length of the variable identifier (at least AFAICT). I'd prefer a fixed indent a lot more.
jvanbruegge
on 14 Jul 2019
oh, and I also agree, single line let and record value assignment is the same, so break if longer than 80 (including indentation) would be the right call here
jvanbruegge
on 14 Jul 2019
Re blank lines between lets, would keeping the layout add much complexity? E.g. I often keep together definitions that are contextual, e.g.:
let a = ..
let b = ..
let c = ..
let d = ..
f-f
on 14 Jul 2019
Regarding the let blank lines you could reintroduce them by using multiple multi-let:
let a = ..
let b = ..
in
let c = ..
let d = ..
in
basile-henry
on 14 Jul 2019
@jvanbruegge: The indent currently doesn't depend on the length of the identifier. The indent is set to two spaces after the beginning of the identifier:
let␣longIdentifierName =
␣␣␣␣␣␣someExpression
in␣␣anotherExpression
The reason why the indent appears to be more than two spaces is because of the use of // and :, so your expression is indented like this:
let someIdentifier =
␣␣␣␣␣␣someExpression
␣␣␣␣⫽ anotherExpression
␣␣: typeAnnotations
in result
... so the first line of the expression is indented 6 spaces, but the expression as a whole (including the type annotation) is only indented 2 spaces.
Gabriel439
on 14 Jul 2019
However, what I could do is make the maximum column parametrizable instead of hardcoding it to 80 characters.
Like a flag to dhall format? If so, I'm not too hot on this change. Many formatters these days have followed the pattern of limiting customization with the consensus being that it's a good decision. I know there's not an explicit goal to follow trends in the industry at large, but it would be nice to follow this one.
A different hardcoded number seems fine though, as it would still remain consistent across all Dhall files formatted with that version.
joneshf
on 14 Jul 2019
The single line records are fine from my side, and I would also prefer a hardcoded number (80 is fine). I just dont like the line wrapping in multiline records
jvanbruegge
on 15 Jul 2019
I’d prefer the formatter not to have any options. You don’t want everybody to paint their bikeshed a different way when it comes to formatting.
Some people will hate it, some will love it, and it doesn’t matter, really.
Profpatsch
on 9 Sep 2019
Related issues
ocharles
·
4Comments
SiriusStarr
·
5Comments
jwoudenberg
·
6Comments
DrSensor
·
5Comments
aljce
·
4Comments