On Tue, Sep 02, 2008 at 04:31:52PM -0700, Jason Dagit wrote:
> David,
>
> I apologize for throwing such large patches your way but I don't think
> I can break them down anymore than this. Also, most of the hunks are
> just changes to indentation. I've tried to be the least invasive I can
> be. Which brings me to another point, this is just the first pass at
> using RIO. Eventually we could replace 'Repository p' in functions
> with RIO and not even export getRepository.
Yes, that would be nice (eventually).
Note that you can often avoid changes to indentation by schemes like
- do foo
- bar
+ rIO $
+ do foo
+ bar
It's not the nicest formatting (and takes an extra line), but that could be
fixed up later. Just for future reference.
> Thanks,
> Jason
>
> Tue Sep 2 16:23:31 PDT 2008 Jason Dagit <[EMAIL PROTECTED]>
> * Add Repository IO monad, RIO.
>
> Tue Sep 2 16:24:58 PDT 2008 Jason Dagit <[EMAIL PROTECTED]>
> * Begin using RIO
Note that I'm not commenting on the good stuff, which is most of this
code.
> +instance Functor (RIO p C(r u t t)) where
> + fmap f m = RIO $ \r -> fmap f (unsafeUnRIO m r)
I think we could safely make this
instance Functor (RIO p C(r u t t1)) where
fmap f m = RIO $ \r -> fmap f (unsafeUnRIO m r)
since it's only modifying the output.
> +-- | Similar to the @ask@ function of the MonadReader class.
> +-- This allows actions in the RIO monad to get the current
> +-- repository.
> +-- FIXME: Don't export this. If we don't export this
> +-- it makes it harder for arbitrary IO actions to access
> +-- the repository and hence our code is easier to audit.
> +getRepository :: RIO p C(r u t t) (Repository p C(r u t))
> +getRepository = RIO return
Yay for this fixme.
> hunk ./src/Darcs/Arguments.lhs 1102
> drop_dotslash x = x
>
> list_unregistered_files :: IO [String]
> -list_unregistered_files = withRepository [] $- \repository ->
> - do s <- slurp_all_but_darcs "."
> - skip_boring <- boring_file_filter
> - regs <- slurp_pending repository
> - return $ map drop_dotslash $ (skip_boring $ list_slurpy s) \\
> (list_slurpy regs)
> +list_unregistered_files = withRepository [] $ do
> + repository <- getRepository
> + rIO $ do s <- slurp_all_but_darcs "."
> + skip_boring <- boring_file_filter
> + regs <- slurp_pending repository
> + return $ map drop_dotslash $ (skip_boring $ list_slurpy s) \\
> (list_slurpy regs)
So this is your general plan, to leave most of the code in IO and only
gradually convert? (Sounds reasonable)
> hunk ./src/Darcs/Arguments.lhs 1111
> - (map drop_dotslash . list_slurpy) `fmap` (withRepository []
> slurp_pending)
> + withRepository [] $ do
> + r <- getRepository
> + s <- rIO $ slurp_pending r
> + return $ (map drop_dotslash . list_slurpy) s
And code like this will be simplified (one line removed, and one variable,
'r') when we move slurp_pending into RIO...
> -add_cmd opts args = withRepoLock opts $- \repository ->
> - do cur <- slurp_pending repository
> +add_cmd opts args = withRepoLock opts $ do
> + repository <- getRepository
> + rIO $ do
> + cur <- slurp_pending repository
> origfiles <- map toFilePath `fmap` fixSubPaths opts args
Do you know why we don't seem any longer to need the $- operator? Is this
something that's going to bite our ghc 6.4 users?
> hunk ./src/Darcs/Commands/AmendRecord.lhs 31
> import Darcs.Lock ( world_readable_temp )
> import Darcs.RepoPath ( toFilePath )
> import Darcs.Hopefully ( PatchInfoAnd, n2pia, hopefully, info )
> -import Darcs.Repository ( withRepoLock, ($-), withGutsOf,
> +import Darcs.Repository ( withRepoLock, withGutsOf, rIO, getRepository,
> get_unrecorded, get_unrecorded_unsorted, slurp_recorded,
> tentativelyRemovePatches, tentativelyAddPatch,
> finalizeRepositoryChanges,
> sync_repo, amInRepository,
> hunk ./src/Darcs/Commands/AmendRecord.lhs 116
> amendrecord_cmd origopts args =
> let opts = if NoTest `elem` origopts then origopts else Test:origopts
> edit_metadata = has_edit_metadata opts in
> - withRepoLock opts $- \repository -> do
> - files <- sort `fmap` fixSubPaths opts args
...
> + withRepoLock opts $ do
> + repository <- getRepository
> + rIO $ do
> + files <- sort `fmap` fixSubPaths opts args
I wonder if we could avoid the excessive indentation here by something like
withRepoLock opts $ getRepository >>= $ \repository ->
do files <- sort `fmap` fixSubPaths opts args
I'm not sure of precedence...
I guess that's all. I only paged throught the rest of the changes, as it
looked like more of the same (and with all the indentation, it would be
hard to spot any real changes).
When I get to work I'll see how cleanly it applies with the other patches
in the queue, and hopefully apply.
David
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users
