[darcs-devel] [patch2155] rebase cleanups

Ben Franksen bugs at darcs.net
Tue Apr 6 12:39:28 UTC 2021


Ben Franksen <ben.franksen at online.de> added the comment:

>>> Minor nitpick: I wouldn't have removed the forall in the type definition
>>> of toRebaseChanges as these often turn out to be needed again in the
>>> future.
>>
>> But you have to admit that the end result is less cluttered now, and
>> thus easier to read.
> 
> I guess the foralls are generically clutter, but I'm fairly used to them
> now. They have the advantage of making the type variable ordering
> explicit for TypeApplications and that defaulting to having them there
> reduces churn.

I agree that these advantages exist. Nevertheless, IME it is usually
(though not always) possible to restructure the code such that explicit
type applications become unnecessary; and similarly with pattern type
signatures. I have done so in many places in Darcs and invariably I find
that the result is simpler and easier to read. So I tend to regard them
as indicators for code that may profit from a bit of refactoring.

That said, explicit foralls are also needed when we delegate the
implementation to a local function that has to have a type signature
because of varying witnesses, e.g. when we traverse RLs or FLs with an
accumulating argument. This happens more often than I would prefer,
which is because with witness types it is not so easy to factor out all
the interesting recursion patterns to higher order functions. Indeed, I
would say that this is the greatest disadvantage of the more precise
typing we get with the witnesses.

Anway, I can agree on keeping existing foralls in type signatures even
when they are no longer strictly needed (except in cases where the type
signature has to be largely re-written anyway).

>>> Also the indentation of the reformatted signature isn't
>>> resistant to changes in the name of the function.
>>
>> I merely followed the convention we use almost everywhere else. I agree
>> that this convention is not ideal for the reason you stated and I am
>> open to a change here. For type signatures that don't fit on a single
>> line you seem to prefer
>>
>> fun
>>   :: (Constraints)
>>   => x
>>   -> y
>>
>> I could adopt that style. Are you using an auto-formatter? I have used
>> hindent for a long time, but it has lots of bugs and often the
>> formatting is not quite what I want so I have to amend the result
>> manually. I have just now installed brittany and it seems to work a lot
>> more reliably and does indeed format type signatures in that way.
> 
> No, I'm not using an auto-formatter. Maybe I should. I don't feel that
> strongly about a particular format but I would like one that allows
> renames easily.

Agreed. I will adopt the above style, especially since it happens to
coincide with what brittany does by default.

I do like auto-formatters, but never use them on complete files, only
selected sections of code; either code I have newly written or
(occasionally) old code with severely messed up indentation. Also, I
always look at the result and only keep it if it is an improvement to
readability; or, if the result is mostly okay, I may keep it and
manually reformat the pieces I don't like.

Stylish-haskell is very well suited for cleaning up import lists, and
perfectly matches our conventions (with the .stylish-haskell.yaml I
added a while ago). I also used hindent for declarations and
expressions, which I have now replaced with brittany for declarations,
expressions, and type signatures.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch2155>
__________________________________


More information about the darcs-devel mailing list