Some cleanups made while trying to figure out how to get at patch info in prehooks.
(Answer: you can't currently, prehooks are run before parsing that stuff.)

4 patches for repository http://darcs.net:

Fri Oct  1 14:58:02 PDT 2010  Simon Michael <simon@joyful.com>
  * hook env vars: add some haddocks

Fri Oct  1 15:01:49 PDT 2010  Simon Michael <simon@joyful.com>
  * hook env vars: clarify some conditional code

Fri Oct  1 15:56:02 PDT 2010  Simon Michael <simon@joyful.com>
  * hook env vars: rename defineChanges/definePatches for clarity

Fri Oct  1 16:29:09 PDT 2010  Simon Michael <simon@joyful.com>
  * hook env vars: haddock improvement

Attachments

• hook-env-vars_-add-some-haddocks.dpatch
• unnamed

> Fri Oct  1 14:58:02 PDT 2010  Simon Michael <simon@joyful.com>
>   * hook env vars: add some haddocks

> Fri Oct  1 16:29:09 PDT 2010  Simon Michael <simon@joyful.com>
>   * hook env vars: haddock improvement

These two look fine and I've pushed them.

> Fri Oct  1 15:01:49 PDT 2010  Simon Michael <simon@joyful.com>
>   * hook env vars: clarify some conditional code

I don't understand the point of this - you've moved some code that's 
only used under #ifndef WIN32 outside the scope of that guard. Won't 
that just make warnings on WIN32?

> Fri Oct  1 15:56:02 PDT 2010  Simon Michael <simon@joyful.com>
>   * hook env vars: rename defineChanges/definePatches for clarity

This depends on the previous patch so is stuck for now. Otherwise it's 
fine.

Thanks for the review Ganesh. Hm, you may be right, I'll check. I  
moved it because the code read as if setEnvCautiously was used only  
within its #ifdef block, which tripped me up when refactoring. And  
secondly because it seems like a utility that's not windows-specific  
and advisable to use elsewhere. I could add an annotation to silence  
this warning ? Otherwise redo the two patches I guess.

On Oct 9, 2010, at 8:18 AM, Ganesh Sittampalam wrote:
> I don't understand the point of this - you've moved some code that's
> only used under #ifndef WIN32 outside the scope of that guard. Won't
> that just make warnings on WIN32?


On Oct 1, 2010, at 5:11 PM, Simon Michael wrote:
> [hook env vars: clarify some conditional code
> Simon Michael <simon@joyful.com>**20101001220149
> Ignore-this: 950ea31d46c94c3cb9df8905c3e86503
> ] hunk ./src/Darcs/Arguments.lhs 1580
>                       finishedOneIO k "DARCS_FILES"
>                       setEnvCautiously "DARCS_FILES" (unlines$  
> listTouchedFiles ps)
>                       endTedious k
> -
> --- | Set some environment variable to the given value, unless said  
> value
> --- is longer than 10K in which case do nothing.
> -setEnvCautiously :: String -> String -> IO ()
> -setEnvCautiously e v | toobig (10*1024) v = return ()
> -                     | otherwise = setEnv e v True
> -    where toobig :: Int -> [a] -> Bool
> -          toobig 0 _ = True
> -          toobig _ [] = False
> -          toobig n (_:xs) = toobig (n-1) xs
> #else
> definePatches _ = return ()
> #endif
> hunk ./src/Darcs/Arguments.lhs 1593
> defineChanges _ = return ()
> #endif
>
> +-- | Set some environment variable to the given value, unless said  
> value
> +-- is longer than 10K characters, in which case do nothing.
> +setEnvCautiously :: String -> String -> IO ()
> +setEnvCautiously e v | toobig (10*1024) v = return ()
> +                     | otherwise = setEnv e v True
> +    where toobig :: Int -> [a] -> Bool
> +          toobig 0 _ = True
> +          toobig _ [] = False
> +          toobig n (_:xs) = toobig (n-1) xs
> +
> posthookCmd :: DarcsOption
> posthookCmd = DarcsMultipleChoiceOption
>                [DarcsArgOption [] ["posthook"] PosthookCmd

On Wed, 13 Oct 2010, Simon Michael wrote:

> On Oct 9, 2010, at 8:18 AM, Ganesh Sittampalam wrote:
>> I don't understand the point of this - you've moved some code that's
>> only used under #ifndef WIN32 outside the scope of that guard. Won't
>> that just make warnings on WIN32?

> Thanks for the review Ganesh. Hm, you may be right, I'll check. I moved it 
> because the code read as if setEnvCautiously was used only within its #ifdef 
> block, which tripped me up when refactoring. And secondly because it seems 
> like a utility that's not windows-specific and advisable to use elsewhere. I 
> could add an annotation to silence this warning ?

It's actually (not-windows)-specific, i.e. it is used on every platform 
but Windows. I guess darcs doesn't set env vars at all on Windows for some 
reason? Would be nice to understand why.

I think it's best to put it back where it was, but I don't mind silencing 
the warning if you prefer - have to do it for the whole file using {-# 
OPTIONS_GHC -f<something> #-}.

> Otherwise redo the two patches I guess.

Since we're now using a 'screened' repository and I already pushed the 
patches there, we need a followon rather than an amendment.

Cheers,

Ganesh

This patch bundle (with 4 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-937cd3b0432f60acb3b9c78c02205b719deade03

I pushed this bundle along with a rollback of the "clarify some 
conditional code" patch.

This patch bundle (with 4 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-937cd3b0432f60acb3b9c78c02205b719deade03