On Fri, Aug 15, 2008 at 12:27:28AM -0700, Dmitry Kurochkin wrote:
> Fri Aug 15 11:09:43 MSD 2008  Dmitry Kurochkin <[EMAIL PROTECTED]>
>   * Resolve issue823: do not exit on keyboard interrupt when getting patches.
>   And give a chance for go_to_chosen_version to run.

Looks good! I have just one small suggestions (below)

Content-Description: A darcs patch for your repository!
> 
> New patches:
> 
> [Resolve issue823: do not exit on keyboard interrupt when getting patches.
> Dmitry Kurochkin <[EMAIL PROTECTED]>**20080815070943
>  And give a chance for go_to_chosen_version to run.
> ] hunk ./src/Darcs/Commands/Get.lhs 57
> +import Darcs.SignalHandler ( catchInt )
> hunk ./src/Darcs/Commands/Get.lhs 173
> -  withRepository opts $- \repository -> do
> -   if format_has HashedInventory rf || format_has HashedInventory rfsource
> -     then do debugMessage "Identifying and copying repository..."
> -             identifyRepositoryFor repository repodir >>= copyRepository
> -             when (SetScriptsExecutable `elem` opts) setScriptsExecutable
> -     else copy_repo_old_fashioned repository opts repodir
> +  copy_repo `catchInt` (putInfo $ text "Interrupted!")

I think that rather than printing "Interrupted!" I'd want to print
something like "Using lazy repository." to make it clear that the ^C
was interpreted as the message (printed earlier) suggests, as a
request for a lazy repo.

> hunk ./src/Darcs/Commands/Get.lhs 176
> +      where copy_repo =
> +                withRepository opts $- \repository -> do
> +                  if format_has HashedInventory rf || format_has 
> HashedInventory rfsource
> +                     then do debugMessage "Identifying and copying 
> repository..."
> +                             identifyRepositoryFor repository repodir >>= 
> copyRepository
> +                             when (SetScriptsExecutable `elem` opts) 
> setScriptsExecutable
> +                     else copy_repo_old_fashioned repository opts repodir
> hunk ./src/Darcs/SignalHandler.lhs 22
> -                             catchNonSignal, tryNonSignal, stdout_is_a_pipe 
> ) where
> +                             catchInt, catchNonSignal,
> +                             tryNonSignal, stdout_is_a_pipe ) where
> hunk ./src/Darcs/SignalHandler.lhs 29
> -import Control.Exception ( catchDyn, throwDynTo, block )
> +import Control.Exception ( catchDyn, throwDyn, throwDynTo, block )
> hunk ./src/Darcs/SignalHandler.lhs 109
> +catchInt :: IO a -> IO a -> IO a
> +catchInt job handler =
> +    job `catchSignal` h
> +        where h s | s == sigINT = handler
> +                  | otherwise   = throwDyn (SignalException s)

I think I might also prefer this to be called catchInterrupt, or maybe
catchSigINT.  The meaning of catchInt isn't quite obvious to me.

Applied.  Thanks!

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

Reply via email to