The system function passes commands to execute via cmd.exe while rawSystem
finds the binary and passes it the arguments.
The difference is seen in:
http://www.haskell.org/ghc/docs/6.10.3/html/libraries/process/System-Process.html#t%3ACmdSpec

The difference between system and rawSystem is in the cmdspec argument
to CreateProcess
system passes cmdspec = ShellCommand while rawSystem passes cmdspec =
RawCommand.

Since 'edit' is a cmd.exe command and not a binary in MS Windows we
can't use
rawSystem to call it because it will result in a not found file error
from CreateProcess.

Using system to call other editors will not work because if they are not
found
an ExitFailure 1 will be raised instead of ExitFailure 9009, 127, 126 or
an exception.

So the easy solution to this problem is to use system only for calling
'edit'
and use rawSystem for all other editors so the chaining with
ortryrunning works correctly.

1 patch for repository http://darcs.net/screened:

Sun Feb 19 20:59:07 <8C>rodkowoeuropejski czas stand. 2012  Adam Wolk
<netprobe@gmail.com>
  * resolve issue2120

Attachments

• resolve-issue2120.dpatch

Well this patch might have been too hasty.
It seems that rawSystem will always fail as it wants a full binary path
to the files.

System.Process.system "vim" -- success on my system as I have vim.
System.Process.rawSystem "vim" -- fails as it's not the full path to vim.

I have no idea at this point how to correctly solve this problem.

Probably should base on some of the internals in
http://hackage.haskell.org/packages/archive/process/1.0.1.3/doc/html/src/System-Process-Internals.html
to expand a command.

I found a way to get the error level from a cmd.exe executed command.

Now the problem is that system from ghc uses only the /c flag and
doesn't append the exit propagation.

C:\Users\mulander>cmd /v:on /c "edi & exit !errorlevel!"
Nazwa 'edi' nie jest rozpoznawana jako polecenie wewnętrzne lub zewnętrzne,
program wykonywalny lub plik wsadowy.

C:\Users\mulander>echo %errorlevel%
9009

Now the problem is in how to call System.Process.rawSystem without
System.Process.Internals.translate breaking the quotes around "command &
exit !errorlevel!"

I can't force haskell to run the raw string "cmd.exe /v:on /c command &
exit !errorlevel!"

I tried with:
 - runProcess
 - system
 - rawSystem
Unfortunately they all go through createProcess and a
System.Process.Internals.translate step.
This breaks the quoting around "command & exit !errorlevel!" so I can't
propagate the error code up to haskell.

This really seems like an implementation bug on rawSystem from ghc as it
does not retrieve the correct exit codes itself (though I agree that
it's not the default behavior for cmd.exe) and prevents any callers from
doing the same (by using translate).

I also don't know how portable this solution would be to command.com.

It seems that the solution was right under my nose. Managed to force the
exit value using the regular system call and adding a new handler to
ortryrunning.
--
The `system' function passes commands to execute via cmd.exe (or
command.com) it's return value
is equivalent to the one returned by the shell.
For regular applications - this works correctly resulting in the exit
code of the program.
However in case of a command/file which can't be found - cmd.exe will
return 1
instead of propagating the ExitFailure 9009 which on windows is
equivalent to
ExitFailure 127 from *nix machines.

Here we force return the exit code of the last cmd.exe action by
appending & exit !errorlevel!
to the command being executed that way chaining with ortryrunning works
correctly.

Attachments

• resolve-issue2120.dpatch

mulander, 2012-02-19.22:36:38 <- This is the version up for review
(second file).

Feel free to remove the initial patch - it was wrong.

I was informed that this approach might not work on systems previous to
Windows 7 and Windows Vista (included).

The reason being that the delayed variable expansion (!variable! instead
of %variable%) is turned on via a registry setting or by passing /v:on
to cmd.exe. It is supposedly turned on by default since Windows Vista/7.

I wanted to pass /v:on to system (and other commands) as noted in the
initial version of the call but this happened to be impossible using
system itself.

As seen in ghc source:
commandToProcess (ShellCommand string) = do
  cmd <- findCommandInterpreter
  return (cmd, translate cmd ++ "/c " ++ string)

the system command utilizes the above function which passes "/c" as the
first argument to cmd.exe. /v:on would have to come *before* /c in other
cases the /v:on will be treated like a disk volume name making the whole
command fail.

Using rawSystem would allow specifying the correct order but I had
trouble constructing the "command & exit !errorlevel!" command because
`translate' is used on the string and that breaks the mandatory quoting.

tl;dr - needs to be tested it if works by default on XP.
In the worst case though - this will have no effect and will result in
the issue being fixed for vista/7 users.
There should be no risk of breaking stuff for older MS Windows releases.

More info on the setting.
It should be possible to set it on by using setlocal before the exit call.

http://ss64.com/nt/delayedexpansion.html
http://ss64.com/nt/setlocal.html

I'm preparing a final patch using SETLOCAL EnableDelayedExpansion just
to make sure it works like expected on systems that don't have it
enabled by default.

Final version using SETLOCAL EnableDelayedExpansion - should work more
reliably on older OS versions.
Additionally I redirected stderr to NUL in order to hide the redundant
messages from each editor that darcs 
tried to run.

--
The `system' function passes commands to execute via cmd.exe (or
command.com) it's return value
is equivalent to the one returned by the shell.
For regular applications - this works correctly resulting in the exit
code of the program.
However in case of a command/file which can't be found - cmd.exe will
return 1
instead of propagating the ExitFailure 9009 which on windows is
equivalent to
ExitFailure 127 from *nix machines.

Here we force return the exit code of the last cmd.exe action by
appending & exit !errorlevel!
to the command being executed that way chaining with ortryrunning works
correctly.

SETLOCAL EnableDelayedExpansion makes sure that !variable! expansion is
done correctly
on systems where that function is not enabled by default.

Attachments

• resolve-issue2120.dpatch

I removed old versions of the patch. The only one left is the final
solution ready for a review.

This breaks issue1465_ortryrunning.sh :-( I haven't had a chance to 
investigate why yet. Are you in a position to run the tests yourself and 
have a look?

In my patch, the following line redirected output from running the
editor to dev/null in order to avoid 6-7 lines of garbage output about
the different editors darcs tried to run but failed to find them. The
output was internationalized I found it more annoying than useful.

cmd ++ " " ++ arg ++ " 2>NULL " ++ -- stderr redirected to NULL

20:02 <Heffalump> no, I think that's catching stdout+stderr
20:03 <Heffalump> ok, it does print the error
20:03 <Heffalump> I guess it's just much more common to have the env
variable set on Linux
20:04 <mulander> yes
20:04 <mulander> default on nix machines
20:04 <mulander> not default on windows
20:04 <mulander> most apps default to notepad on windows
20:04 <mulander> or use some registry setting for file mappings
20:05 <Heffalump> s likely it'll fail on the first editor it tries
20:05 <mulander> also it's rare for a nix machine not to have a 'vi'
20:05 <Heffalump> perhaps the right answer is to change the defaults for
windows
20:05 <Heffalump> i.e. make notepad work as per other discussion, and
make that the first tried
20:06 <mulander> that would make sense.
20:06 <mulander> generally revmap runEditor and getEditor
20:06 <mulander> getEditor should return vi on *nix and notepad on ms win
20:06 <mulander> edit can be left where it is as a last resort
20:06 <mulander> and we can drop the output redirection
20:07 <mulander> no need to add additional debug information.
20:07 <mulander> the default is more sane for the platform
20:07 <mulander> and less likely to chain executions of things that have
little chance of being there
20:07 <mulander> and if you're using emacs/vim then you are more than
likely to know how to set an env var.
20:07 <mulander> if that's an acceptable solution
20:08 <mulander> I would follow up patch 724 with a change that removes
the output redirect
20:08 <mulander> and do the other changes in the issue I registered
issue2144

2 patches for repository http://darcs.net/screened:

Mon Feb 20 01:15:49 Œrodkowoeuropejski czas stand. 2012  Adam Wolk
<netprobe@gmail.com>
  * resolve issue2120

Thu Feb 23 20:36:49 Œrodkowoeuropejski czas stand. 2012  Adam Wolk
<netprobe@gmail.com>
  * follow-up issue2120 : don't redirect stderr as it loses useful output
  
  Redirecting stderr breaks issue1465_ortryrunning.sh test case.
  The reason behind the failure is the way that script checks for success
  and failure. The check is based on running egrep on stderr/stdout output
  and checking if the expected editor binary names appeared in the output.
  Since I redirected stderr - the expected binary names were not included in
  the output expected in that script.
  
  The other drawback of redirecting stderr is the possibility of hiding
  useful information about actual application failures.
  
  This patch reverts both regressions by not redirecting the output.
  
  Redirection was done in the first place because on MS Windows on a system
  that does not have any environment variables set for editors (common case)
  darcs outputted 8 lines of internationalized output about a missing
command
  or file trying to be run.
  The additional output is not a problem on different platforms as it's
really
  rare for a system to not have 'vi' or having one of the environment
variables
  set to a sane value.
  
  The solution to this problem will be provided as a patch for issue2144.
  notepad.exe will be the default for MS Windows platforms hence the
chaining
  will rarely happen so the additional redundant output will stop being
  a problem.
  
  Until a solution to issue2144 is provided this patch still should be
applied
  since without it darcs fails to run any editors on a default MS Windows
  installation - including not being able to run 'edit'.

Attachments

• resolve-issue2120.dpatch

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

This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/screened.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_screened.html#bundle-62ee9626d58f519100750eb3c69df48bbadfa45e

Thanks

This patch bundle (with 1 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-bf3714f32037ab147cf1f3a4671ffda942937b4c

This patch bundle (with 2 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-62ee9626d58f519100750eb3c69df48bbadfa45e