[darcs-devel] darcs patch: Depends.lhs: remove duplicated code from... (and 4 more)

[David Roundy]

On Thu, Apr 07, 2005 at 07:46:45PM +0200, Tomasz Zielonka wrote:
> Hello!
> 
> Here are a couple of patches related to get_common_and_uncommon_or_missing
> and get_extra. The first two are not rather uncontroversial (unless you
> don't like the names of new functions), they simply simplify things.
> 
> The third one removes one case from get_extra I think was unnecessary -
> when "skipped" is empty, we simply commute the patch with an empty
> patch.

I guess the question here is whether calling commute with an empty patch is
bad.  I guess since we have to read mp anyways, there's no real reason to
avoid the commute.  So I tentatively agree.  I think that special case was
there from before I added the throwing of an error, and it used to allow us
to lazily delay the reading of the patch.

> The reason for fourh and the fifth patch: recently I often found myself
> applying patches meant for darcs-unstable to darcs-stable. As the stable
> repository lacks some patches from unstable, I was unable to apply these
> patches. However, most often than not I was getting an uninformative
> error from get_extra rather that a nice error message from "apply".
> In tests on small repositories I was getting either a nice error from
> apply, or an uninformative error - this depended on the contents of both
> repositories.
> 
> These uninformative messages turned out to be exceptions (or bottom) thrown
> using errorDoc form pure code of get_common_and_uncommon_or_missing /
> get_extra. When the value computed by get_common_and_uncommon_or_missing
> was evaluated deeply enough, the error would pop up. Sometimes apply_cmd
> was able to see that it is missing some patches before it touched
> bottom/exception.
> 
> I think that we should rather handle such errors explicitly and I've
> added new function get_common_and_uncommon_or_missing which allows the
> caller to see that some patch was missing. Its type is:
> 
>  (PatchSet,PatchSet) -> Either PatchInfo ([PatchInfo],PatchSet,PatchSet)
> 
> I also had to change the type of get_extra. I've tried not to change the
> semantics of it.
> 
> The fifth patch adds use of this function in apply_cmd.

These look good to me!
-- 
David Roundy
http://www.darcs.net