Undesired(?) extra spacing around a '.' in a type signature #19

Open
opened 2017-04-09 22:23:58 +02:00 by barrucadu · 4 comments
barrucadu commented 2017-04-09 22:23:58 +02:00 (Migrated from github.com)

Should this space be added? It's not a style I think I've seen before.

runTest
  :: Predicate a
   -- ^ The predicate to check
-  -> (forall t. ConcST t a)
+  -> (forall t . ConcST t a)
   -- ^ The computation to test
   -> Result a
runTest test conc = runST (runTestM test conc)
Should this space be added? It's not a style I think I've seen before. ```diff runTest :: Predicate a -- ^ The predicate to check - -> (forall t. ConcST t a) + -> (forall t . ConcST t a) -- ^ The computation to test -> Result a runTest test conc = runST (runTestM test conc) ```
lspitzner commented 2017-04-09 23:44:23 +02:00 (Migrated from github.com)

Ah, this seems like a nice purely stylistic question :) and I have no strong opinion on this really. My preference leans towards the current behaviour, but the reasoning is weak. I think the strongest argument is the consistency with "=>" and "->" which might be considered higher-precedence operators inside the type signature, but forall is syntax already, so: I am not opposed to changing the default here, generally.

A quick grep analysis in the ghc-7.10.3 bootlibs looks like this:

> grep "forall[^.]*[^ ]\." -r bootlibs | wc -l
328
> grep "forall[^.]* \." -r bootlibs | wc -l
266

(so 328 without, 266 with space). For some somewhat older stackage snapshot, it looks more in favour of omitting the space:

> grep "forall[^.]*[^ ]\." stackage-snapshot | wc -l
5175
> grep "forall[^.]* \." -r stackage-snapshot | wc -l
1447

Two question:

  1. How about the "forall" in existentials? would you prefer data Foo = forall x. X x => Foo x as well?

  2. The multi-line version is fine? E.g.

restartingModuleNetwork
  :: forall t m
   . (RH.MonadAppHost t m, R.Reflex t, MonadIO (R.PushM t))
  => [ModuleStatic]
  -> Trigger ModuleEvent
  -> R.Event t ModuleEvent
  -> m
       ( R.Dynamic t (Map ModuleId ModuleState)
       , R.Event t (ProcessScript ())
       )

And of course it is trivial to make this configurable (although i doubt that many would bother to change the default).

Ah, this seems like a nice purely stylistic question :) and I have no strong opinion on this really. My preference leans towards the current behaviour, but the reasoning is weak. I think the strongest argument is the consistency with "=>" and "->" which might be considered higher-precedence operators inside the type signature, but `forall` is syntax already, so: I am not opposed to changing the default here, generally. A quick grep analysis in the ghc-7.10.3 bootlibs looks like this: ~~~~ > grep "forall[^.]*[^ ]\." -r bootlibs | wc -l 328 > grep "forall[^.]* \." -r bootlibs | wc -l 266 ~~~~ (so 328 without, 266 with space). For some somewhat older stackage snapshot, it looks more in favour of omitting the space: ~~~~ > grep "forall[^.]*[^ ]\." stackage-snapshot | wc -l 5175 > grep "forall[^.]* \." -r stackage-snapshot | wc -l 1447 ~~~~ Two question: 1) How about the "forall" in existentials? would you prefer `data Foo = forall x. X x => Foo x` as well? 2) The multi-line version is fine? E.g. ~~~~.hs restartingModuleNetwork :: forall t m . (RH.MonadAppHost t m, R.Reflex t, MonadIO (R.PushM t)) => [ModuleStatic] -> Trigger ModuleEvent -> R.Event t ModuleEvent -> m ( R.Dynamic t (Map ModuleId ModuleState) , R.Event t (ProcessScript ()) ) ~~~~ And of course it is trivial to make this configurable (although i doubt that many would bother to change the default).
barrucadu commented 2017-04-10 00:39:30 +02:00 (Migrated from github.com)

I prefer keeping the dot with the type variables, because unlike => and -> it's not conceptually a type-level operator. This would make the multi-line version :: forall t m. as well.

I prefer keeping the dot with the type variables, because unlike `=>` and `->` it's not conceptually a type-level operator. This would make the multi-line version `:: forall t m.` as well.
int-index commented 2017-04-10 00:50:24 +02:00 (Migrated from github.com)

Please keep the space. With DependentHaskell we will have both forall t m . and forall t m ->. If we want the space for the arrow, we should keep it for the dot.

Please keep the space. With `DependentHaskell` we will have both `forall t m .` and `forall t m ->`. If we want the space for the arrow, we should keep it for the dot.
tfausak commented 2019-06-18 19:49:41 +02:00 (Migrated from github.com)

I'm in favor of keeping the space. It's not clear to me if we can reach a consensus or if this should be configurable.

I'm in favor of keeping the space. It's not clear to me if we can reach a consensus or if this should be configurable.
Sign in to join this conversation.
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#19
There is no content yet.