On Tue, Aug 19, 2008 at 8:54 AM, David Roundy <[EMAIL PROTECTED]> wrote:
> On Tue, Aug 19, 2008 at 08:46:31AM -0700, Jason Dagit wrote:
>> On Tue, Aug 19, 2008 at 8:23 AM, David Roundy <[EMAIL PROTECTED]> wrote:
>> > On Fri, Aug 15, 2008 at 02:52:41PM -0700, Jason Dagit wrote:
>> >> On Fri, Aug 15, 2008 at 2:30 PM, David Roundy <[EMAIL PROTECTED]> wrote:
>> >> > I can see two simple workarounds for this:  one would be to simply
>> >> > pattern match on (h_us:<:NilRL) and leave any other case in the
>> >> > "impossible" category.  The other would be to simply use concatRL,
>> >> > which in this case is equivalent to headRL, except in type.  I think
>> >> > the former possibility is slightly cleaner, as it documents the
>> >> > stupidity in the interface.  But if my analysis is wrong and there
>> >> > really is a bug, then you won't have fixed the bug, you'll just have
>> >> > made it exit with an error message.
>> >>
>> >>
>> >> Even if I use the patten match above, we have the problem that
>> >> considerMergeToWorking needs to know that both patch sequences start from
>> >> the unrecorded state.  So, I'm left wondering how to extract that proof 
>> >> from
>> >> get_common_and_uncommon.  I feed get_common_and_uncommon two sequences, us
>> >> :: C(() r), and them :: C(() a).  Actually, I'm not confident that them
>> >> should be a patchset.  I hava feeling it's supposed to be them :: C(a b)
>> >> when I look at how it was constructed.  But if that's the case then it
>> >> wouldn't even be safe to call get_common_and_uncommon.  It's created by
>> >> reading the repository's unrevert then doing a scan_bundle on that to
>> >> convert to a PatchSet, but I find it hard to believe that scan_bundle
>> >> returns a patchset.  It seems more likley that scan_bundle returns an RL 
>> >> (RL
>> >> p)) C(x y), and it's up to the caller to tell scan_bundle the true meaning
>> >> of those contexts.
>> >
>> > Okay, one of your questions is easily answered, which is that
>> > scan_bundle does correctly return a PatchSet, it just returns a lazy
>> > patchset that doesn't have all of its elements.  The key is that the
>> > patch bundle includes the context.  Looking at the code, I just
>> > noticed that we don't actually return a proper lazy PatchSet in
>> > scan_bundle, which I should definitely fix (perhaps after the big push
>> > that I'm doing right now).
>>
>> Oh okay.  I'll try to fold this in with the changes below.
>
> Actually, I've written this change (lazier scan_bundle) already, and
> am just waiting a bit (to see if the resulting darcs works all right
> on at least a few applies) before pushing.

It's really nice to see the type wit work paying off.  I have the
following list of things that we improved as a result, aside from the
obvious like type safe manipulations and tracking of
recorded/unrecorded/tentative state for repositories:

* with_selected_paches_from_repository bad commute
* interactive changes command context problem
* scan_bundle wasn't lazy enough
* get_choices supersedes similar but lossy separate_* functions
* removed broken/unused rempatch from one of the commands
* improved core function get_common_and_uncommon
* simplified commute_to_end in Depends.lhs
* removed commute_by in SelectChanges

I'm trying to make a good list so we have nice cite-able examples to
write about.  If you can think of something I missed let me know.

Thanks,
Jason
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to