[darcs-users] darcs patch: Use index for diffing in the basic whats... (and 6 more)

[Eric Kow]

On Mon, Jun 01, 2009 at 11:34:28 +0200, Petr Rockai wrote:
> This bundle comes with patches to fix most of the issues we currently have with
> indexed whatsnew. Sorry to shuffle the code again, but I think we are now
> getting there. More detailed reply to the review separately, later.
 
> Eric writes:
> > - boring file (Reinier, Petr: regression?)
> Fixed.
> 
> > - pending renames (Petr: regression?)
> Fixed.
> 
> > - Darcs.Gorsvet witnesses (Eric: blocker for application)
> To be done (next bundle).

Cool, so it looks like combined with your other patch that fixes your
witnesses, we're now ready to try pushing this!

---------------------------------------------------------------------
Use index for diffing in the basic whatsnew scenario.
Pass options to unrecordedChanges and handle LookForAdds and IgnoreTimes.
Fix the path restriction versus pending renames in unrecordedChanges.
---------------------------------------------------------------------
already reviewed

Factor out a separate boring_regexps in Darcs.Repository.Prefs.
---------------------------------------------------------------
Looks good.

I think you could have named boring_regexps as boringRegexps if you
wanted.

http://wiki.darcs.net/CodingStyle on case

> hunk ./src/Darcs/Repository/Prefs.lhs 269
> -    return $ actual_boring_file_filter $ map mkRegex (bores++gbs)
> +    return $ map mkRegex $ bores ++ gbs
> +
> +boring_file_filter :: IO ([FilePath] -> [FilePath])
> +boring_file_filter = boring_regexps >>= return . actual_boring_file_filter

actual_boring_file_filter `fmap` boring_regexps ?

Provide a restrictBoring (like restrictSubpaths) in Darcs.Gorsvet.
------------------------------------------------------------------
> +restrictBoring = do
> +  boring <- boring_regexps
> +  let boring' (AnchoredPath (Name x:_)) _ | x == BS.pack "_darcs" = False
> +      boring' p _ = not $ any (\rx -> isJust $ matchRegex rx p') boring

What about paths that include _darcs in them (subrepos?)

Fix Tree restriction in various cases of unrecordedChanges.
-----------------------------------------------------------
> +  nonboring <- restrictBoring

>                 (False, True) -> do guide <- unfold current
> -                                   restrict guide `fmap` readPlainTree "."
> +                                   all <- readPlainTree "."
> +                                   return $ restrict_ $ (restrict guide) all

> -               (True, _) -> filter nodarcs `fmap` readPlainTree "."
> +               (True, _) -> do all <- readPlainTree "."
> +                               return $ restrict_ $ nonboring all

Makes sense to me...

Take a list of paths in unrecordedChanges instead of Tree transform.
--------------------------------------------------------------------
It's a bit hard to tell what's going on just from the diff, so here's
the function:

This patch adds this function (factored out from unrecordedChanges) which
returns pending patches that touch a list of files.  The filenames it accepts
are the current unrecorded names.

| pendingChanges :: (RepoPatch p) => Repository p C(r u t)
|                -> [SubPath] -> IO (Sealed (FL Prim C(r)))
| pendingChanges repo paths = do
|   slurp_pending repo -- XXX: only here to get us the "pending conflicts" check
|                      -- that I don't know yet how to implement properly
|   Sealed pending <- read_pending repo
|   let files = map (fn2fp . sp2fn) paths
|       pre_files = apply_to_filepaths (invert pending) files
|       relevant = case paths of
|                    [] -> seal pending
|                    _ -> choose_touching pre_files pending
|   return relevant

OK, so now I understand why you want to invert pending.  It's so that you can
get the recorded filename equivalent to the working filename, so that you can
correctly pick out the patches to that file.

The rest of this looks good.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090601/db5a2dfe/attachment.pgp>