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
signature.asc
Description: Digital signature
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
