[darcs-devel] darcs patch: Rename AtExit module to Global. (and 2 more)

Tommy Pettersson ptp at lysator.liu.se
Sat Aug 19 17:40:57 PDT 2006


I've looked at these patches now and if they work as they are
meant to I suppose they're fine.

>   This implementation relies on a notion of global variables using
>   unsafe IORefs.

> setSshControlMasterDisabled :: IO ()
> setSshControlMasterDisabled = writeIORef _sshControlMasterDisabled True
> 
> {-# NOINLINE sshControlMasterDisabled #-}
> sshControlMasterDisabled :: Bool
> sshControlMasterDisabled = unsafePerformIO $ readIORef _sshControlMasterDisabled

I have not looked up what IORefs are, but
sshControlMasterDisabled is using unsafePerformIO; can we really
be sure it is not evaluated before any
setSshControlMasterDisabled? I guess we can, since it would be
lazily evaluated in darcs commands run from consider_running,
within IO, after any setSshControlMasterDisabled.

I am still in favor for a state monad to carry around all
options, read from files, command line or environment variables.
Then it would be clear from the type declaration which functions
changed their behavior according to customizations. And we
wouldn't have to pass the opt arg around, parsing it again and
again possibly in slightly different ways by accident. But then
again, this might affect almost every little function in darcs.
I remember it was an unpleasant prospect to carry IO all the way
through every level of the Doc printers just to have some
control over colors in the end. That's why it uses
unsafePerformIO.

I think this fix will do nicely. Unless anyone objects I'll
apply it to darcs stable soon.


-- 
Tommy Pettersson <ptp at lysator.liu.se>




More information about the darcs-devel mailing list