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 -->
_______________________________________________
darcs-devel mailing list
[email protected]
http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
