Hi Alexander,
On 06.11.2017 02:51, Alexander Coffin wrote:
Fixes: https://bugs.winehq.org/show_bug.cgi?id=32679
This version (v3) is now completely c89 compliant. The previous had a one line issue that was not.
Before "&&" and "||" were supported, but they also didn't check if past commands had failed. This commit removes direct access to errorlevel. Now errorlevel can only be accessed using getters and setters, but the it seemed like the most efficient solution. Also please note I added one TODO task. Currently this commit doesn't replicate a certain windows bug with setting errorlevel to 1 in some cases. See test_builtins.cmd for more details.
I would have checked the success/failure of commands that don't set the errorlevel if that was possible, but Winows doesn't do that. It doesn't care about the most recent statement, it only cares about the errorlevel if it changes, otherwise it assumes success (if the errorlevel doesn't change).
I must admit that depending on the fact if errorlevel changed looks questionable to me. You probably mostly want to handle cases of successes that don't end up clearing errorlevel, but there are other ways to do that. While looking for info about this, I found a very nice answer on Stackoverflow:
https://stackoverflow.com/questions/34936240/batch-goto-loses-errorlevel/349...
What I like about this is that it covers everything with tests (although I didn't verify results myself). It seems to prove that the actual success of commands is separated from errorlevel and should be handled separately. I think it would make sense to preserve actual command result and handle it separately from errorlevel. Then we could use that to decide on failure/success in || and && operators.
Implementing that is a bit more work than this patch. I think we'd want WCMD_execute to return int representing executed command return code (that means that WCMD_if, WCMD_for, WCMD_move, ECMD_run_program and alike will also have to do it). Once you have that, you can simply use returned value to properly handle && and || operators. How does that sound to you?
Thanks, Jacek