rakitzis / rc

rc shell -- independent re-implementation for Unix of the Plan 9 shell (from circa 1992)
Other
250 stars 23 forks source link

Modified the behavior of assignment to not modify the status code. #83

Closed gtnoble closed 1 year ago

gtnoble commented 1 year ago

This is consistent with the behavior of bash/POSIX shell.

Before, it was impossible (to me) to get the status code when making an assignment from a command substitution.

Now you can have really neat C-like code like this:

if ( response=`{make-http-request $url} ) {
        echo $response
} else {
        echo 'an error occurred' >[1=2]
        echo $response
}
borkovic commented 1 year ago

Bash script

/tmp/ $ cat asg
echo $BASH_VERSION
false; x=3; echo $?
x=$(false); echo $?
x=3 false; echo $?

Running bash script

/tmp/ $ bash asg
3.2.57(1)-release
0
1
1

Script for current rc

/tmp/ $ cat  asg.rc
echo $version
false; x=3; echo $status
x=${false}; echo $status
x=3 false; echo $status

Running rc script

/tmp/ $ rc asg.rc
1.7.4 $Release: @(#)rc 1.7.4 glob-linear-213-g1da327d $
0
0
1

I believe that this change will result in all 3 cases setting $status to 1, but for bash in the first case status ($?) is 0.
Is this discrepancy important?
Can you think of any other construct involving an assignment?

gtnoble commented 1 year ago

You are correct:

echo $version
false; x=3; echo $status
x=`{false}; echo $status
x=3 false; echo $status
1=3; echo $status

result:

gtnoble@gtnoble-XPS-13-9305 ~/S/rc ; rc assignment_test.rc 
1.7.4 $Release: @(#)rc 1.7.4 v1.7.4-117-g8ca9ab1 $
1
1
1
rc: line 5: numeric variable name

Is this discrepancy important?

I don't think it really matters from my point of view, because assignment errors will crash shell scripts. So, there is no point in checking the status of the assignment itself in a script.

Interactively, the status is set for errors.

; 1=3
rc: numeric variable name
; echo $status
1
xyb3rt commented 1 year ago

I'm willing to merge this if the behaviour matches that of the bash shell. false; x=3; echo $status should result in 0 because x=3; is a full command and should set $status accordingly. The trick is to not overwrite $status if the assignment succeded and the value came from a backquote substitution.

xyb3rt commented 1 year ago

Changing the patch to

diff --git a/walk.c b/walk.c
index 4b67475..584e784 100644
--- a/walk.c
+++ b/walk.c
@@ -169,8 +169,8 @@ top:        sigchk();
        case nAssign:
                if (n->u[0].p == NULL)
                        rc_error("null variable name");
-               assign(glom(n->u[0].p), glob(glom(n->u[1].p)), FALSE);
                set(TRUE);
+               assign(glom(n->u[0].p), glob(glom(n->u[1].p)), FALSE);
                break;
        case nPipe:
                dopipe(n);

Has the desired effect:

; false; x=3; echo $status
0
; x=`{false}; echo $status
1
; x=3 false; echo $status
1

But does not make it fully behave like bash:

$ x=$(false) y=1; echo $?
1

In rc:

; x=`{false} y=1; echo $status
0
oliwer commented 1 year ago

FYI, Plan 9 rc does not set $status on assignments either.

gtnoble commented 1 year ago

FYI, Plan 9 rc does not set $status on assignments either.

What is the full behavior of the Plan 9 rc in the above test cases? I think this would be a better target than POSIX or bash shell.

oliwer commented 1 year ago

@gtnoble Using the port from https://github.com/benavento/rc

% false; x=3; echo $status
1
% x=`{false}; echo $status
1
% x=3 false; echo $status
1
% x=`{false} y=1; echo $status
1

This makes sense if you consider an assignment is not a command.