[darcs-devel] Re: [darcs-users] Log messages by e-mail

[Nathaniel Gray]

David Roundy wrote:
> On Mon, Jul 24, 2006 at 10:18:23PM -0700, Nathaniel Gray wrote:

>> Ok, now we're on darcs-devel so we can talk code.  I suppose a sensible 
>> design would be to have each command return a list of string pairs 
>> representing additions to the posthook environment.  For the moment, 
>> they would all return [] except for apply, which would return something 
>> like [("darcs_patch_count", "7")].  Let me know if there's another 
>> approach you'd prefer.
> 
> Sounds reasonable to me, but it'd be nice to also add something to the
> framework to document which environment flags are set, and to arrange
> things so the documentation (provided by --help) is always guaranteed
> to be correct.  And to make sure that the commands behave in a unified
> manner.  I'm thinking that we could perhaps achieve this in a
> moderately friendly way with a:
> 
> data PostHookEnvironmentVariable = NUMBER_PATCHES | ChangeLogEntry | PhaseOfMoon
>      deriving ( Show )
> 
> and then add to DarcsCommand an entry like
> 
> data DarcsCommand =
>     DarcsCommand {command_name, command_help, command_description :: String,
>                   ...
>                   command_posthook_envs :: [PostHookEnvironmentVariable],
> 
> which would list the defined environment variables, and then we'd have
> 
>                   ...
>                   command_command :: [DarcsFlag] -> [String] -> IO [(PostHookEnvironmentVariable,String)],
> 
> which is equivalent to what you describe, but command --help can be
> written to automatically include which post-hook env variables are
> included.  

This sounds sensible.  I would be in enthusiastic agreement, but I just 
finished changing 35 commands to have types
[DarcsFlag] -> [String] -> IO [(String,String)]
so you'll have to settle for a somewhat strained smile.  ;^)  Actually, 
I should be able to hack my own patch to rework that change pretty 
painlessly.

> We can document the meaning of each variable in only one
> place, with a function of type
> 
> document_post... :: PostHookEnvironmentVariable -> String
> 
> If this isn't possible, perhaps we should have different variable
> names (i.e. you shouldn't reuse a posthook env variable to mean
> different things for different darcs commands).

How about this:  We define a central datatype for all env vars, and the 
command_posthook_env field of the DarcsCommand record is a list of 
(PostHookEnvironmentVariable, String) pairs, where the string is a 
description of what the variable will be set to.  The question of shared 
vs. unique names is orthogonal, provided you can live with 
PushPatchCount being defined in DarcsCommands.lhs instead of Push.lhs.

> Then we could also have the command-running code verify at runtime
> when running the posthook that the command provides all the promised
> environment variables.  It's not as nice as a static constraint, but I
> don't see how we can do this statically.  But still, a runtime check
> should catch almost any errors.
> 
> The important thing is that we need to ensure that our post-hook
> environment variables are properly documented, particularly since I
> forsee that once we get started, we're likely to end up adding quite a
> few of them.

I agree.  I'm all in favor of code-generated documentation.

>> I see run_posthook in Test.lhs, which looks like the place to begin, and 
>> there's also consider_running in DarcsCommands.lhs.  Hopefully the type 
>> checker will make it clear how to proceed from that point.  Guess it's 
>> time to deal with the frustration of being an "expert beginner" all over 
>> again...
> 
> Indeed, that sounds like a good starting point, and yes, the type
> checker is your best friend.

So far so good.  I've almost got the version I proposed working, and the 
changes you've suggested shouldn't be too much work, I hope.

One last thing.  I'm working on the code from darcs 1.0.8.  Is that 
going to be a problem?  Should I update to the current devel version?

Thanks,
-n8

-- 
 >>>-- Nathaniel Gray -- Caltech Computer Science ------>
 >>>-- Mojave Project -- http://mojave.cs.caltech.edu -->