Command Line, Modify Var and the "=" Character

[cmerighi]

trafficstars

https://github.com/less/less.js/blob/2d43c0b9648daee7f226f1be9952654c80dea033/bin/lessc#L73-L76

To me, the above lines optimistically consider that no string variables containing an "=" char will ever be passed.
Considering the scenario where:

• in a less file, I define a selector variable:

@shell_selector: "html";
@shell: (~"@{shell_selector}");

@{shell}{
    /* ... */
}

• and in a command line:

lessc --modify-var="shell_selector='html[theme=dark]'" src/less/styles.less dist/css/styles-dark.css

The latter fails. The error message includes the line that cannot be interpreted by the parser: @shell_selector: 'html[theme; which is the result of the referenced lines of code on top.

[kevinramharak]

I wonder what te motivation for the second argument of split('=', 2) is. The docs state it is a limiter and notes:

The left-over text is not returned in the new array.

// split with one argument
'var=html[data-attr="value"]'.split('='); // ["var", "html[data-attr", ""value"]"]`
// split with two arguments
'var=html[data-attr="value"]'.split('=', 2); // ["var", "html[data-attr"]

I would propose a change of the highlighted snippet to:

var parseVariableOption = function(option, variables) { 
     var parts = option.split('=');
     variables[parts[0]] = parts.slice(1).join('='); 
 };

as

const parts = 'var=html[data-attr="value"]'.split('='); // ["var", "html[data-attr", ""value"]"]`
parts.slice(1).join('='); // "html[data-attr="value"]"

[cmerighi]

I see many possible amendments here:

var parseVariableOption = function(option, variables) { 
    let arr = /^([^=]+)=(.+)$/.exec(option);
    if (arr && arr.length > 2){
        variables[arr[1]] = arr[2];
    }
}

(Regex could be refined to match the only characters allowed for a variable name in Less.js. Something like ^([a-zA-Z_$][\w$-]*)=(.+)$ ?)

[kevinramharak]

That is actually a good idea, might as well add some simple validation logic. But what happens on invalid input?

[cmerighi]

That is actually a good idea, might as well add some simple validation logic. But what happens on invalid input?

Invalid input gets silently skipped in the example above. It may throw if it's the case:

var parseVariableOption = function(option, variables) { 
    let arr = /^([a-zA-Z_$][\w$-]*)=(.+)$/.exec(option);
    if (arr && arr.length > 2){
        variables[arr[1]] = arr[2];
    } else {
        throw `Couldn't parse '${ option }' as a valid name-value pair.`;
    }
}

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cmerighi]

I think it's worth keeping this issue alive...

[matthew-dean]

@cmerighi I fully agree that this command-line parsing is way to simplistic and naive. I'd like to see argument parsing replaced by a lightweight NPM library for argument parsing.

I also think this form in the Less documentation is clunky: lessc --plugin=my-plugin=args The weird nested chaining of equals assignments doesn't make sense, and I'd like to see something that's more consistent with other CLI tools that support plugins.

Would people in this thread be able/willing to: a) come up with a lightweight library for lessc argument parsing b) recommend a new syntactic structure for Less 4.0?

I also would like to see Less plugins export a function (e.g. export default (args) => {install() {...}} vs export default {install() {...}}, and then Lessc could, when a function is exported, pass in parsed args vs just the raw argument string that needs parsing by the plugin, and then deprecate the setOptions method described here: http://lesscss.org/features/#plugin-atrules-feature-the-less-js-plugin-object. (Note: my thinking is that setOptions would still receive the raw argument string for back-compat.)

This would be more consistent with other plugin-capable ecosystems, where each plugin is a function that can optionally receive an options object.

There's a lot of moving parts for Less, so this feature needs a champion (or champions) to just figure out the best way to move forward.