Jason Dagit wrote:
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.
Yes, the issue is that system doesn't accept an environment. There's
the option of putting the variables into darcs' environment, but AFAIK
the way to do that is System.Posix.setEnv, which is described as
non-portable.
I would *love* to continue using system -- parsing command lines is no
fun at all. A workaround would be as easy as passing the command to "sh
-c" if it weren't for Windows...
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.
You'll have to correct me if any of this is wrong, since this is my
first experience with Haskell, but I don't think records are the way to
go. Using records means every command has to supply every field. Why
should "darcs add" need to care about patchesApplied? It's bad enough
that all the possible env vars have to be defined in one place -- it
would be much nicer if common vars were defined together and vars that
only one command will supply were defined with that command, but I'm not
sure how to achieve that elegantly in Haskell.
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.
I'll leave the discussions of indentation to those who have to live with
the code. Suffice to say it all looks weird to me. ;^)
Cheers,
-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
