[darcs-devel] [patch1725] moved inOrderTags from convert command t... (and 2 more)

[Ben Franksen]

>>   * avoid access to PatchSet constructors where possible
> 
>> hunk ./src/Darcs/UI/SelectChanges.hs 1013
>> +  _ :> x <- return $ contextPatches pps
>> hunk ./src/Darcs/UI/SelectChanges.hs 1015
>> -  FlippedSeal ps <-
>> -    return $ case pps of PatchSet _ x -> FlippedSeal (x+>>+(pa:>:NilFL))
>> -  let my_lps = labelPatches Nothing ps
>> +  let my_lps = labelPatches Nothing (x+>>+(pa:>:NilFL))
> 
> This isn't obviously behaviour-preserving, given that
> contextPatches does a slightlyOptimizePatchset. Can you
> explain it a bit more?

You are right. The change means we won't be offered any patches beyond a
tag if that tag happens to be clean but darcs hasn't (yet) recognized
that. While previously we would be offered such patches up to the last
tag that was (already) recognized as clean. Note that all commands that
change the repo will do slightlyOptimizePatchset before changes are
finalized. So the only situation where this could happen is when we
unsuspend multiple patches with --ask-deps, one of which is a tag and we
want a later unsuspended patch to depend on a patch that comes before
that tag.

Anyway, I don't think this matters much. I am not sure what the
motivation was in limiting the selection to "loose" patches in the first
place. It makes no sense to me and I think it unnecessarily restricts
the use of --ask-deps. But if we take it as given, then surely /after/
we recorded or amended the patch in question, a slightlyOptimizePatchset
will have been done. So we wouldn't be able to add that dependency in a
new amend, no matter what. Which means that the behavior is now more
consistent than before.

I think the code should be fixed, but so as to allow to choose any patch
from the past, getting rid of the contextPatches call.