This bundle also contains a few dependent or closely related refactors.

8 patches for repository http://darcs.net/screened:

patch b988f00b57857b669afa36894f426c3eaf01daa1
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Jun  9 14:18:25 CEST 2018
  * removed unsused (incl. darcsden) function getFirstMatchS

patch 3063340b61ba373179c81e9b11e7e2d8de97e9e1
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Jun  9 16:23:56 CEST 2018
  * simplify D.P.Match.matchExists

patch b0213b98827f0348e37912b48e5b6e82942368d4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jun 10 00:19:46 CEST 2018
  * inlined a where clause in splitOnTag

patch 1e2aafcf172be023e2a1657f2346e0ba4a19d4a4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jun 10 00:22:32 CEST 2018
  * make spanRL lazy, add takeWhileRL

patch c931f4c777a2ebdb7504ef5519e0516bc50560e4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jun 10 00:23:50 CEST 2018
  * refactor matching of PatchSets
  
  This affects the commands annotate, clone, diff, dist, show contents, and
  show files. For these, selecting a single patch or a range of patches
makes
  no sense; instead matching means to select the version (PatchSet)
consisting
  of all patches up to (including) the latest matching patch, except for
--tag
  where we get the exact version corresponding to the tag.
  
  The functions findAPatch and matchPatch and the data type
  InclusiveOrExclusive are now obsolete have been removed. Other
functions are
  superseded: getPatchesBeyondTag is replaced by the more general
  splitOnMatchingTag; getTagS and getMatcherS have been inlined into
  getNonrangeMatchS, which has been renamed to rollbackToPatchSetMatch;
  havePatchsetMatch is replaced by patchSetMatch. The new data type
  PatchSetMatch precisely captures the different ways to match a PatchSet;
  patchSetMatch returns it and rollbackToPatchSetMatch and
getOnePatchSet take
  it as argument. Also, getNonrangeMatch is now called getRecordedUpToMatch,
  and dropn is renamed to patchSetDrop and exported.
  
  There are now separate DarcsFlags and MatchFlags for --index=N (OneIndex)
  versus --index=N-M (MatchIndexRange), which streamlines index matching.

patch 579f43b7647b30149b511cf296724d65bc053d7d
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 11 12:24:59 CEST 2018
  * simplify return type of splitOnTag
  
  Instead of returning a PatchSet with an empty trailing list of patches
plus
  trailing patches separately, we as well return just a PatchSet and put the
  trailig patches back.

patch a9349a5a5fca24d09607ab2a49ec914d3cb7b216
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 11 19:11:39 CEST 2018
  * move matchingHead to D.P.Match, move contextPatches to D.P.Depends

patch c22d0154593adfdc417aa7e48f8ddece62926de8
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 11 19:35:02 CEST 2018
  * move filterNonInternal from tag command to D.P.Match

Attachments

• removed-unsused-_incl_-darcsden_-function-getfirstmatchs.dpatch

>  * removed unsused (incl. darcsden) function getFirstMatchS

Fine

>  * simplify D.P.Match.matchExists

> -matchExists _ (PatchSet NilRL NilRL) = False
> -matchExists m (PatchSet (ts :<: Tagged t _ ps) NilRL) = matchExists m (PatchSet ts (ps:<:t))
> -matchExists m (PatchSet ts (ps:<:p)) | applyMatcher m p = True
> -                                     | otherwise = matchExists m (PatchSet ts ps)
> +matchExists m = not . unseal nullRL . dropWhileRL (not . applyMatcher m) . patchSet2RL

I think this would be simpler still with 'anyRL' or 'allRL'.

In general with this kind of refactoring I worry about laziness - if we read too much I think
it might fail/require network fetches on lazy repositories. I can't spot any problems, but
it'd be easy to miss some subtlety. Ideally we'd have tests at the command level that could
catch laziness problems.


>  * inlined a where clause in splitOnTag

Fine

>  * make spanRL lazy, add takeWhileRL

OK (it would be nice to have a test explaining the desired laziness property, to stop someone
refactoring it back)

>  * refactor matching of PatchSets

I don't understand the matching argument handling well enough to really check this very thoroughly
but it seems sane (and an improvement) to me so I'm fine with it.

>  * simplify return type of splitOnTag

Looks like a good simplification
  
>  * move matchingHead to D.P.Match, move contextPatches to D.P.Depends

Fine

>  * move filterNonInternal from tag command to D.P.Match

Fine

>>  * make spanRL lazy, add takeWhileRL
> 
> OK (it would be nice to have a test explaining the desired laziness
> property, to stop someone refactoring it back)

See attached bundle.

>>  * simplify D.P.Match.matchExists
> 
>> -matchExists _ (PatchSet NilRL NilRL) = False
>> -matchExists m (PatchSet (ts :<: Tagged t _ ps) NilRL) = matchExists
m (PatchSet ts (ps:<:t))
>> -matchExists m (PatchSet ts (ps:<:p)) | applyMatcher m p = True
>> -                                     | otherwise = matchExists m
(PatchSet ts ps)
>> +matchExists m = not . unseal nullRL . dropWhileRL (not .
applyMatcher m) . patchSet2RL
> 
> I think this would be simpler still with 'anyRL' or 'allRL'.

Right, but note that the later patch 'refactor matching of PatchSets'
removes this function, which means the point is moot now...

Attachments

• document-lazyness-of-spanrl_-breakrl_-takewhilerl.dpatch