[darcs-devel] [patch1887] remove the size prefix when writing hash... (and 14 more)

[Ben Franksen]

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

> A further thought about the RepoType parameter: I can't see any use to
> keeping it on PatchInfoAnd, given that as you say we no longer drag
> a containing rebase patch in it. But I think it may have some value
> on Repository, as there is a difference between a rebase-in-progress
> repository and a "normal" one. I don't know whether that value
> outweighs the cost.

Regarding the cost/benefit ratio: I don't mind dragging around the 'rt'
type parameter so much. What I do mind is that actually using it (either
at runtime or at compile time) incurs this ridiculous amount of
syntactic overhead. It makes type signatures longer and harder to read
and the case distinctions themselves are riddled with complicated extra
type annotations and singleton type parameters. The fact that working
with singleton types in Haskell can be rather awkward has been
recognized even by experts that are otherwise ardent supporters of
dependent typing.

I am a personally big fan of Haskell's strong static type system and I
appreciate very much how it enables safe refactoring using some of the
more advanced features like phantom ("witness") types. I guess it
contributes a large part of the fun I have with hacking on darcs that I
/can/ do these large refactors and still get it working afterwards
without weeks of debugging.

But there are limits. If the typing overhead gets to the point of
obscuring code that is basically a plain and simple case distinction,
then IMO that limit is reached. We should also keep in mind that any
advanced type shenanigans make it harder for newcomers to contribute.
And finally, trying to capture ever more things at the type level isn't
the only, and perhaps not the best, way of making progress: re-writing
complicated procedures so that it becomes obvious what they do and why,
commenting anything that remains tricky, adding haddocks, consistent and
succinct but still suggestive names for variables etc etc..., in other
words improving code quality. This is what I have been doing for years
now, and I think it has proven quite effective.

Your use of RepoType when the rebase patch was mixed with normal patches
was (barely) justified (though it really gave me quite a headache to
understand it at first): it was important that we don't accidentally mix
rebase-in-progress patches with normal ones.

Nowadays, there is no longer any essential difference between a
rebase-in-progress repository and a "normal" one. We still make a
distinction when we start a RepoJob but I think that is no longer
needed. So what if we use e.g. 'darcs rebase unsuspend' or 'darcs rebase
log' and there is no rebase in progress? Nothing bad would happen at
all. This could be handled just like we handle other "empty" cases i.e.
simply report that there are no suspended patches after reading the
rebase patch.

We could even drop the final "Rebase in progress: N suspended patches"
message; I find it a bit distracting since almost all of my repos
nowadays have a few suspended patches. But I'm surely not the average
user, and I don't mind if we keep it. (It should certainly be reported
if we act on my proposal for an enhanced 'darcs status' command.)

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