On 8/2/06, Eric Y. Kow <[EMAIL PROTECTED]> wrote:
Some initial comments:
On Wed, Aug 02, 2006 at 18:14:40 -0700, Nathaniel Gray wrote:
> Aside from some aesthetic objections, the only serious problem I see is
> that changing the method used to execute the posthook from
> System.Cmd.system to System.Process.runProcess will break any posthook
> command that counts on being processed by the shell.
I don't like this new restriction. Every posthook I've ever written
depended on the shell. It's not the end of the world obviously, but
I'd rather not add this restriction. What is the technical reason for
this? Is it because you couldn't pass the new environment with
System.Cmd.system? If that's the case, could you try adding the
environment variables in darcs's process before calling
System.Cmd.system? It may be the case that the command inherits the
environment of darcs. This would also need to be checked by someone
on windows as it may not be portable.
Another issue is that System.Process.runProcess only showed around GHC
6.4(.1?), whereas we're supporting an earlier version, the one in Debian
stable.
What would probably be the right thing to do is to edit Workaround.hs so
that it exports runProcess. If we are using GHC 6.2, runProcess could
likely call Exec.exec. Anyway, in the long run, it would be nice if we
switched to using runProcess everywhere.
Why? Because of the environment passing? Perhaps what we need is to
look at the code for System.Cmd.system and add something to
Workaround.hs that implements System.Cmd.system but accepts an
environment.
On the other hand... maybe it would be useful in general if darcs
commands returned some information instead of just exiting.
1. What would you all think if we had a mechanism like this, only
renamed from PostHookEnvironmentVariable to DarcsCmdStatus,
and implemented as a record (instead of an attribute value list)
? See below.
I'm with you so far. In sort of a "oh gee, that's really obvious, why
isn't it that way already..." way.
2. What else might we use it for?
data DarcsCmdStatus = DarcsCmdStatus { patchesApplied :: Maybe Int
, cmdExitCode :: ExitCode }
(I can never decide on the best indentation for these things)
Due to scoping of the record names I've picked up another habbit
(which people here probably won't like). So I'd go a step further and
use this:
data CmdStatus = CmdStatus { cmdStatus'PatchesApplied :: Maybe Int
, cmdStatus'ExitCode :: ExitCode
}
I'm not sure if the formatting will look right when I send this.
Basically, I line up the comma with the opening and closing bracket,
line up the type signatures and then use the funny names.
Jason
_______________________________________________
darcs-devel mailing list
[email protected]
http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
