rakitzis / rc

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

if not doesn't handle multiple preceding ifs or nested if nots [PATCH] #74

Closed ptommy closed 1 year ago

ptommy commented 2 years ago

Example:

if (false) echo FALSE    # This truth value will be used by ifnot
if (true) echo TRUE
if not echo NOT TRUE

This erroneously outputs NOT TRUE.

I should mention that this is a rather serious bug, if you're using if not. It can make the logic of a script completely wrong if you are using a simple if without else or if not early in the script, and later use if-if not, where the if not will use the truth value of the earlier if and not the preceding one.

Here's a patch that tests for this in trip.rc

diff --git a/trip.rc b/trip.rc
index 43af11b..6ac5fe0 100644
--- a/trip.rc
+++ b/trip.rc
@@ -713,6 +713,7 @@ submatch 'if (false) { echo foo } else echo qux; if not echo bar' 'rc: `if not''
 # if-not stack if_state regression
 s='
 L=()
+if (false) l=($L a0)
 if (true) {
     if (false) {
         L=($L  a1)

And here's a fix:

diff --git a/walk.c b/walk.c
index 314484b..50581a6 100644
--- a/walk.c
+++ b/walk.c
@@ -102,8 +102,8 @@ top:        sigchk();
                cond = TRUE;
                if_this = walk(n->u[0].p, TRUE);
                cond = oldcond;
-               if (if_last == if_nothing) if_last = if_this;
                walk(if_this ? true_cmd : false_cmd, parent);
+               if_last = false_cmd ? if_nothing : if_this;
                break;
        }
        case nIfnot: {

if_last should be updated after possibly nested if-ifnots are executed, and it should be set to the truth value of an else-less if, or to if_nothing if the if had an else.

This will trigger the error "if not must follow if" whenever ifnot is used after an else (or an ifnot), but it will not trigger an error for the following:

if (false) echo FALSE
echo ALWAYS
if not echo NOT FALSE

I don't think this necessarily should be considered an error. Right now however the parser does, although it only detects in in lines, so the following will trigger the "if not must follow if" error:

if (false) echo FALSE; echo ALWAYS; if not echo NOT FALSE

but should it...?

xyb3rt commented 1 year ago

I've removed the if not syntax, so this is no longer an issue.