make flagsToPristine obey repository format.
--------------------------------------------
> -flagsToPristine :: [DarcsFlag] -> Pristine
> -flagsToPristine fs | UseFormat2 `elem` fs || UseHashedInventory `elem` fs = 
> HashedPristine
> -flagsToPristine (PristineNone : _) = NoPristine (darcsdir++"/" ++ 
> pristineName ++ ".none")
> -flagsToPristine (PristinePlain : _) = PlainPristine (darcsdir++"/" ++ 
> pristineName)
> -flagsToPristine (_ : t) = flagsToPristine t
> -flagsToPristine [] = flagsToPristine [PristinePlain]
> +flagsToPristine :: [DarcsFlag] -> RepoFormat -> Pristine
> +flagsToPristine _ rf | format_has HashedInventory rf = HashedPristine
> +flagsToPristine (PristineNone : _) _ = NoPristine (darcsdir++"/" ++ 
> pristineName ++ ".none")
> +flagsToPristine (PristinePlain : _) _ = PlainPristine (darcsdir++"/" ++ 
> pristineName)
> +flagsToPristine (_ : t) rf = flagsToPristine t rf
> +flagsToPristine [] rf = flagsToPristine [PristinePlain] rf

Indeed, this appears to be just a refactor, and an improvement.

Now we are only use [DarcsFlag] to distinguish between pristine and no
pristine.  Interestingly (as was the case before), the inventory options
override the pristine flags.  Maybe we could swap the order of the
overrides : we could rename --plain-pristine-tree to just
--pristine-tree (so it could generate hashed pristine) and make the
pristine/no-pristine distinction come first.  Or maybe we could even do
away with the pristine/no-pristine distinction altogether.  Does anybody
actually use --no-pristine?

resolve issue805: make darcs-2 format the default for new repositories.
-----------------------------------------------------------------------
This seems ok to me.  I couldn't think of any other parts of the code
that need changing...

> hunk ./src/Darcs/Arguments.lhs 859
> -                          "Minimal features. What older repos use. 
> [DEFAULT]"]
> +                          "Minimal features. What older repos use."]

We could even stick [DEPRECATED] in there

> -              then UseFormat2:filter (/= UseOldFashionedInventory) opts
> +              then UseFormat2:opts
>                else if format_has HashedInventory rfsource &&
>                        not (UseOldFashionedInventory `elem` opts)
>                     then UseHashedInventory:filter (/= UseFormat2) opts
> hunk ./src/Darcs/Commands/Get.lhs 141
> -                   else filter (/= UseFormat2) opts
> +                   else UseOldFashionedInventory:filter (/= UseFormat2) opts

The idea here seems to be that darcs get calls darcs init, and darcs
init no longer assumes UseOldFashionedInventory as the default, so we'd
better make it explicit!

By the way, instead of calling darcs init, we could now call
createRepository instead.

> hunk ./src/Darcs/Repository/Format.lhs 71
>  create_repo_format :: [DarcsFlag] -> RepoFormat
>  create_repo_format fs = RF ([map rp2ps flags2inv] ++ maybe2)

This looks right.  Only silly style comments below.

> -    where flags2inv | UseFormat2 `elem` fs || UseHashedInventory `elem` fs = 
> [HashedInventory]
> -                    | otherwise = [Darcs1_0]
> +    where flags2inv | UseFormat2 `elem` fs = [HashedInventory]
> +                    | UseOldFashionedInventory `elem` fs = [Darcs1_0]
> +                    | otherwise = [HashedInventory]

It looks like we can simplify this to

    where flags2inv | UseOldFashionedInventory `elem` fs = [Darcs1_0]
                    | otherwise = [HashedInventory]

Alternatively, we could make it more explicit/verbose

    where flags2inv | UseFormat2 `elem` fs = [HashedInventory]
                    | UseHashedInventory `elem` fs = [HashedInventory]
                    | UseOldFashionedInventory `elem` fs = [Darcs1_0]
                    | otherwise = flags2inv [HashedInventory]

Which could simplify switching to some hypothetical inventory format...
or maybe I'm just in YAGNI-land

> -          maybe2 = if UseFormat2 `elem` fs then [[rp2ps Darcs2]] else []
> +          maybe2 = if UseFormat2 `notElem` fs &&
> +                      (UseOldFashionedInventory `elem` fs ||
> +                       UseHashedInventory `elem` fs)
> +                   then []
> +                   else [[rp2ps Darcs2]]

For uniformity, maybe we could use the same guard-oriented-style as
flags2inv?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

Attachment: signature.asc
Description: Digital signature

_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to