Skip to content

Conversation

@iChenLei
Copy link
Member

PR Target

Try to fix #3616

Debug analysis

If you write less (v4 parens-division default) code like follow:

.parens-issues-3616 {
  bar: if(false, 666, 888 / 444);
  bar2: if(false, 666, (666 / 333));
  bar3: if(false, 666, ((444 / 222)));
}

VSCode debug screen snapshot

2021-06-11 21 24 47

Call Stack

ParseTree -> ruleset eval -> declaration eval -> value eval -> expression eval -> call eval -> functionCaller.call -> If -> operation eval

Input Expected Result
bar2: if(false, 666, (666 / 333)); bar2: 2; bar2: 666 / 333;

In our expacted logic the Operation's eval output should be a Unit which value is 2.

(666 / 333) compile to a Expression which contains a Operation

2021-06-11 21 43 39

The Operation's eval logic

   //  packages/less/src/less/tree/operation.js
    eval(context) {
        let a = this.operands[0].eval(context), b = this.operands[1].eval(context), op;

        if (context.isMathOn(this.op)) {   //  <- we get false in this case
             /**** omit unimportant code  ****/
            return a.operate(context, op, b);  // <-  so we can't get expected output from here
        } else {
            return new Operation(this.op, [a, b], this.isSpaced);
        }
    },

Our context lose parens !!!

   //  packages/less/src/less/functions/function-caller.js:29 
        args = args
            .filter(commentFilter)
            .map(item => {
                if (item.type === 'Expression') {
                    const subNodes = item.value.filter(commentFilter);
                    if (subNodes.length === 1) {
                        return subNodes[0];    //  <-  lose Expression and get a Opreation
                    } else {
                        return new Expression(subNodes);
                    }
                }
                return item;
            });

So the context.isMathOn(this.op) is failed.
I know my analysis is not very clear, but I think maintainers know what i say. Hope this PR is useful, maintainers can modify my code directly cause i am not a less source code expert. Any help welcome.

/cc @matthew-dean

@iChenLei
Copy link
Member Author

BTW: I noticed that less still use deprecated travis-ci(travis-ci.org), please migrate to travis-ci.com(migrate to Github Action is also a good choice)

2021-06-11 22 17 10

@matthew-dean
Copy link
Member

@iChenLei Thanks!

@matthew-dean matthew-dean merged commit c518180 into less:master Jun 12, 2021
@iChenLei iChenLei deleted the fix-parens-bug branch June 12, 2021 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

if() requires double parentheses when dividing

2 participants