fix: Output correct css when using css function in max().

[lumburr]

What: fix #3604

Why:

How:
For the following ast node

[Dimension, Expression]

less will skip non-Dimension nodes
https://github.com/less/less.js/blob/master/packages/less/src/less/functions/number.js#L26-L31
So, max(1, calc(1 + 1)) is treated as max(1).

We should not skip these nodes.
Checklist:

• Documentation
• Added/updated unit tests
• Code complete

[iChenLei]

Thanks for contribution.

[matthew-dean]

So, max(1, calc(1 + 1)) is treated as max(1).

That doesn't seem correct. max(1, calc(1 + 1)) should output max(1, calc(1 + 1)) in the CSS. Similar for min(). See other function escaping code for functions like rgb() which output as-is if something like a var() statement is used.

[lumburr]

In rgb((0 128 var(--num)), the program tries to convert var(--number) into a number,Throws an exception if the conversion cannot be performed.

less.js/packages/less/src/less/functions/color.js

Lines 40 to 51 in 1eafc06

	function number(n) {
	if (n instanceof Dimension) {
	return parseFloat(n.unit.is('%') ? n.value / 100 : n.value);
	} else if (typeof n === 'number') {
	return n;
	} else {
	throw {
	type: 'Argument',
	message: 'color functions take numbers as parameters'
	};
	}
	}

So, we also throw an exception on non-Dimension value.
Since the node was not successfully converted, Less will be output as is

[matthew-dean]

@lumburr That's true but this will output as-is if compiled in Less.

.rule {
  color: rgb(0 128 var(--num));
}

So I'm saying, this in Less:

.rule {
  value: max(1, calc(1 + 1));
  }

...should output:

.rule {
   value: max(1, calc(1 + 1));
 }

[lumburr]

Ohh,@matthew-dean I may not have expressed it clearly.
You are completely correct that in Less:

.rule {
  value: max(1, calc(1 + 1));
}

should output

.rule {
  value: max(1, calc(1 + 1));
}

And now the issue is caused because: max(1, calc(1 + 1)) is treated as max(1)

So what this PR does is to make Less output

.rule {
  value: max(1, calc(1 + 1));
}

correctly again.

[matthew-dean]

Oops, can you resolve conflicts?

[bugron]

Hi @lumburr, thanks for your contribution. Is there a chance you'll be able to fix merge conflicts here so that this PR can be merged?

[matthew-dean]

@iChenLei Would you have time to resolve conflicts and add test cases for this issue to see if they are resolved?

[matthew-dean]

Or @lumburr do you have time to do this?

[SoonIter]

@matthew-dean
I've finished this in another pr #4266, could you CR and merge it? 😁