I have fixed some things now, and it looks better
------------------------------
>
> -------------------------
> > + pullingFrom <- mapM (fixUrl opts) repos
> > + withRepoLock opts $- \initialRepository ->
> > + modifyCache initialRepository pullingFrom >>= \repository ->
> > + fetchPatches opts' repos "pull" repository >>= applyPatches opts'
> repository
>
> OK, we still have the two repositories here but at least it's clearer
> which one you should be using at all times.
>
> You could get rid of the extra argument by reversing the definition of
> modifyCache so it lends itself more to partial application.
>
I tried before with partial application, but I was getting each time
"Inferred type is less polymorphic than expected Quantified type variable
`p' escapes", I read a workaround for it in a Oleg's paper, but that will
mean creating a newtype and ... , I think is not work for this case, I used
the lambda instead, I came back to use the do notation, I tried to do it
with binding but the compiler was complaining about a type which was
infering incorrectly, using do notation did the trick.
> +-- | Add locals repositories to cache, ensuring that locals are tried
> before any remote one.
> > +modifyCache :: forall p C(r u). RepoPatch p => Repository p C(r u r) ->
> > + [String] -> IO (Repository p C(r u r))
> > +modifyCache (Repo dir opts rf(DarcsRepository pristine (Ca ccache)))
> repos =
>
> I think modifyCache would be more useful as a higher order function
> (otherwise, I would be not so inclined to have it in the
> Darcs.Repository hierarchy).
>
> Rather than being specifically about adding the local repositories to
> the cache, it could just be about applying some (Cache -> Cache) on the
> repository's cache.
>
>
I modify "modifyCache" now it is located in Repository.InternalTypes, there
is a similar function there which acts over ther cache, so that's why I
decided to put there, also, mornfal noticed :
*18:23 mornfall: InternalTypes is a misnomer already -- but yes, it will do
> for now.*
>
So, I will write these down for a future refactor.
> + do
> > + let cache = Ca (addLocals ccache repos)
> > + return (Repo dir opts rf (DarcsRepository pristine cache))
> > + where
> > + addLocals = foldr (\a b -> if isFile a
> > + then Cache DarcsCache.Repo NotWritable
> a : b
> > + else b)
>
Now the addLocal method looks is clearer, also I moved back to Pull.hs, and
makes use of the function "modifyCache", it looks like :
addLocal repo repos = modifyCache repo $ \ (Ca cache) ->
Ca $ [Cache
DarcsCache.Repo NotWritable r | r <- repos, isFile r ] ++ cache
> +compareByLocality (Cache _ _ x) (Cache _ _ y)
> > + | isFile x && (isUrl y || isSsh y) = LT
> > + | (isUrl x || isSsh x) && isFile y = GT
> > + | otherwise = EQ
>
> Every top level function should have a type signature.
>
>
Sorry, I didn't added it :).. now it has its signature
> copyFileUsingCache :: OrOnlySpeculate -> Cache -> HashedDir -> String ->
> IO ()
> > -copyFileUsingCache oos (Ca cache) subdir f =
> > - do debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir
> subdir)++"/"++f
> > +copyFileUsingCache oos (Ca unsortedCache) subdir f =
> > + do let (Ca cache) = Ca (sortBy compareByLocality unsortedCache)
> > + debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir
> subdir)++"/"++f
>
> Interesting. One question: does this sorting belong here? What if in
> the future we want to sort by proximity to the repository directory?
> How would we manage that? What's the right attitude to adopt here? One
> possible answer, for example, is YAGNI. Is that the right one?
>
> I have lifted all that sorting from there, now it is done when it is as it
is populated ( thanks Petr :))
> fetchFileUsingCachePrivate :: FromWhere -> Cache -> HashedDir -> String
> -> IO (String, B.ByteString)
> > -fetchFileUsingCachePrivate fromWhere (Ca cache) subdir f =
> > +fetchFileUsingCachePrivate fromWhere (Ca unsortedCache) subdir f =
> > do when (fromWhere == Anywhere) $ copyFileUsingCache ActuallyCopy
> (Ca cache) subdir f
> > ffuc cache
> > `catchall` debugFail ("Couldn't fetch `"++f++"'\nin subdir
> "++(hashedDir subdir)++
> > hunk ./src/Darcs/Repository/Cache.
>>
>> hs 265
>> >
>> > ffuc [] = debugFail $ "No sources from which to fetch file
>> `"++f++"'\n"++ show (Ca cache)
>> >
>> > + Ca cache = Ca (sortBy compareByLocality unsortedCache)
>>
>> What's the difference between these two functions?
>
>
I don't have a clear answer for that question, I was writing something but
then I realize it was a supposition I was doing, I hope to have a proper
answer by the weekend :).
--
Adolfo
_______________________________________________
darcs-users mailing list
darcs-users@darcs.net
http://lists.osuosl.org/mailman/listinfo/darcs-users
