[darcs-users] [PATCH, RESEND] Eliminate HopefullyPrivate (rename over Hopefully)

[Jason Dagit]

I think the idea behind the patch sounds fine.  We don't seem to use the
separation between Hopefully and HopefullyPrivate, so it just seems
supurfluous at the moment.  We could always add it back at a later time if
it seems useful then.

More comments below.

On Sat, Aug 9, 2008 at 2:26 PM, Nathaniel W Filardo <nwf at cs.jhu.edu> wrote:

> On Sat, Aug 09, 2008 at 06:51:24PM +0200, Nicolas Pouillard wrote:
> > Excerpts from Simon Michael's message of Sat Aug 09 18:41:33 +0200 2008:
> > > Second thoughts!
> > >
> > > I had trouble understanding and applying this patch. It complained
> about
> > > removing an non-empty file, but then did it anyway. I obliterated the
> > > patch and then noticed my my repo was inconsistent, with Hopefully.lhs
> > > containing 0 bytes. darcs check confirmed, darcs repair fixed it.
> That's
> > > a bad patch.
> >
> > Nathaniel did you use the --look-for-adds option? If so, you've probably
> it a
> > known bug (to lazy to look at the tracker). I traditionally workaround
> this
> > by killing some/all parts of the _darcs/patches/pending: DO IT CAREFULLY,
> or
> > by using darcs repair.
>
> I don't think so, but just in case, I've regenerated the patch (attached)
> --
> this one passes the visual smoke test.
>
> Thinking more about it, though, I don't really understand why Hopefully
> exists.  The data type,
>
> > data Hopefully a C(x y) = Hopefully (SimpleHopefully a C(x y))
> >                         | Hashed  String (SimpleHopefully a C(x y))
>
> seems equivalent to
>
> > SimpleHopefully a C(x y) * Maybe String


We have:
data SimpleHopefully a C(x y) = Actually (a C(x y)) | Unavailable String

Ignoring for a moment the phantom types, it's this:
data SimpleHopefully a = Actually a | Unavailable String

Which looks much closer to "Either String a".  Oh, but maybe I don't
understand your notation?  Does the * mean product and thus you would take
that to mean the Either type in this case?

Now it seems that we have this same structure again with the Hopefully
type.  So in terms of existing types we could do this:

type Hopefully (a C(x y)) = Either (Either String (a C(x y))) (Either
(String, (Either String (a C(x y))))

It should be obvious why we created a new type.  Even if we only create
SimpleHopefully, it's not so pretty:

type Hopefully (a C(x y)) = Either (SimpleHopefully (a C(x y))) (String,
SimpleHopefully (a C(x y)))

Given how easily we can construct our own custom type and then give the
corresponding instances that Either would have, it seems like making a new
data type is best.

Although, it turns out, due to our phantom types, we can't give an instance
of Monad, so we defined something that behaves the same way as the Monad
instance if we didn't have the phantom types.

which should be
>
> > Maybe (a C(x y)) * Maybe String
>
> But, AFAICT, the only reason for the Maybe String seems to be because
> PatchInfo structures don't store the hashed-repository ("new") hash value
> --
> the root complication with issue861.


I can't really comment on this without digging into it.  Maybe it was done
this way to avoid putzing around with all the existing PatchInfo code,
derived instances and what not.


>
>
> Is it silly to think that PatchInfos should, even for old repositories,
> store (potentially unevaluated thunks -- hooray laziness) their new hash
> value?  Am I missing something obvious?


The hash could be a Maybe String.  (Hopefully David) will comment is
(Hopefully (Actually ..)) and not (Hopefully (Unavailable ..)).  :-)

Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osuosl.org/pipermail/darcs-users/attachments/20080810/3b3ad8f5/attachment.htm