Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Semicolon as Function Argument Separator #1505

Closed
matthewwithanm opened this issue Aug 21, 2013 · 16 comments
Closed

Allow Semicolon as Function Argument Separator #1505

matthewwithanm opened this issue Aug 21, 2013 · 16 comments

Comments

@matthewwithanm
Copy link

Since semicolons are the recommended way to separate arguments for mixins, it seems to me that it would be a good idea to allow them to be used for functions as well.

@lukeapage
Copy link
Member

But mixins are a less concept where as functions exist in css (and cannot
be semi-colon seperated).. but interesting point and I'm not dismissing it.

@Soviut
Copy link

Soviut commented Aug 21, 2013

What exactly would be the benefit of having semi-colons as separators?

@matthewwithanm
Copy link
Author

The reason I raised the issue is just for internal consistency—not because it made some new feature possible. The point about the shared syntax with CSS functions is a good one though.

@SomMeri
Copy link
Member

SomMeri commented Aug 23, 2013

@Soviut You could easily pass comma separated lists into them e.g. the same reason why there are semicolons in mixins. There are plans to make functions pluginable so it is not hard to imagine that some people might create lists manipulating functions then. For example, the extract function already does that.

@matthew-dean
Copy link
Member

With the merge of @plugin, this request makes sense. People may reasonably want to pass in CSS lists as an argument to a custom function without having to resort to hackish escaping.

@matthew-dean
Copy link
Member

Should I remove the "Consider Closing" tag? (Maybe consider leaving open?)

@matthew-dean
Copy link
Member

See #2270 for a real use-case example where semi-colon separaters in a user-defined function would be necessary, or at least desirable. (The "each" example passes in a CSS list.)

@chharvey
Copy link
Contributor

chharvey commented May 4, 2016

just a thought…
maybe keep css functions (such as linear-gradient()) per usual with comma-separated arguments, but allow Less functions that aren't present in css (such as fadeout() and mix()) to have semicolon-separated arguments. in addition to being consistent with using semicolon-separated arguments in mixins, this would also help make the distinction between css functions and Less functions.

@matthew-dean
Copy link
Member

matthew-dean commented May 4, 2016

@chharvey What you say sounds logical, but would be a bit of a challenge. In the first stage of evaluation, Less is only parsing your code. Evaluation of functions only happens after you have a complete tree. Which makes sense, if you think about it, because a custom function can receive an entire ruleset, spanning multiple lines and property / value pairs (and arguments).

When functions are evaluated, Less.js checks to see if there's a matching function name within the current scope. If not, it outputs the function as is. When it writes CSS, the operation happens in reverse, with arguments output with comma separators.

However... I think mixin nodes are already marked as having comma or semi-colon separators, so maybe the same could happen for functions. So that: if a Less function is not found during evaluation AND the function has semi-colon separators, then Less could throw an error.

@seven-phases-max
Copy link
Member

seven-phases-max commented May 4, 2016

this would also help make the distinction between css functions and Less functions.

The initial idea was actually to not have distinction between two (just because both CSS and Less function-sets are not something set in stone). So parser does not and cannot have any distinction between those. More over some CSS functions (e.g. rgba) are actually handled via Less functions, and most Less functions (e.g. contrast) fallback to CSS functions when their parameters do not match.

P.S. Ah, @matthew-dean is faster :) But anyway keeping my comment just for "in other words".

@matthew-dean
Copy link
Member

@seven-phases-max Looks like we commented at the same time. The parser wouldn't know if they're valid, but could mark the type of separator.

@seven-phases-max
Copy link
Member

seven-phases-max commented May 4, 2016

So that: if a Less function is not found during evaluation AND the function has semi-colon separators, then Less could throw an error.

Does not look like something useful since it can't cover 0..1-parameter functions anyway. Besides, some (radical purists like me) not just do not favor ; as separators (thus do not share that docs sentence) but actually whenever possible recommend strictly the opposite ("never use ; unless it's unavoidable") :neckbeard:. So as a user I definitely would not want to be forced to use extract(foo bar; 1).

So to target that problem (e.g. throw a error for extract(foo bar, 11) instead of passing as-is, which is unrelated to this FR), could not we just have a compiler option?

@matthew-dean
Copy link
Member

matthew-dean commented May 4, 2016

No, no, not forced. Never forced. The request is actually just as an option. And you misunderstood. Less would not throw an error for extract() with a comma. Not at all. I was suggesting it could throw an error if a) semi-colons were used, b) the function was not found. But such a thing is not a necessity. The feature request is just semi-colon as an option, like it is for mixins. It seems legit, if for no other reason than you could send a comma-separated list to a function, which was the same rationale to add the option to mixins.

@seven-phases-max
Copy link
Member

seven-phases-max commented May 4, 2016

I was suggesting it could throw an error if a) semi-colons were used, b) the function was not found.

Yes, but if I'd want to use that nice new feature ("throw a error if Less function is not found") I would have to use semicolons (and could not get the same with commas) :) But never mind, I get what you mean in general.

@matthew-dean
Copy link
Member

Ohhh okay, now I get what you mean. Ha, yes. This is true. Which is probably why we should pass on that idea. I was just spitballing how Less could distinguish Less/CSS functions, as @chharvey was suggesting, but yeah, it's probably better to avoid that.

@matthew-dean
Copy link
Member

Optional semi-colons in functions are supported in Less 2.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants