Expose Less parsing as a top level feature of the less package

[jackwanders]

Converting a Less stylesheet into an AST is a valuable piece of functionality, and worthy of being easily accessible to consumers of less.js. There have been issues submitted in the past about accessing the AST generated by the LESS parser, and without copy/pasting much of the code found within lib/render.js, it has thus far been a difficult task.

This pull request strips the parsing functionality out of render.js and moves it to parse.js, making it a top level property of the less module. In the same way that less.render takes a less string and returns a css string, less.parse takes a less string and returns a less AST.

I would also like to propose having the option to have extra data attached to this AST (for example, line/column location information for tokens like rules, imports, variables, mixin calls, etc), but before I started working on that, I wanted to get your opinion on this feature first.

Thanks.

[seven-phases-max]

My vision of this is that currently the AST is just too complex for any use outside of the compiler itself (almost every entity there requires all that knowledge of all those specific things Less syntax consists of). So in this context I wonder what would be typical use of the AST in its current state (I can imagine some for earlier days of Less when the AST was young and simple but for now it looks like it's nearly impossible to do something valuable with it w/o duplicating almost half of the compiler code).
Hence personally I sympathized the "semi-deprecation" of that "public" less.parse stuff in v2.

[lukeapage]

Yes, i agree Max. But, also im fine with this refactor - it seems harmless?
one thing is that parse should be passed into render - here you are creating it twice.

[seven-phases-max]

@lukeapage

Yes, I guess the changes in the PR itself are fine on their own. I just wondered of "worthy of being easily accessible to consumers of less.js".

[jackwanders]

@seven-phases-max I could probably word that better as "accessible to developers". I cannot speak for others, but I am currently tinkering with a checkstyle program for less documents, and this PR makes it very straightforward to call less.parse on my stylesheet, and then run a selection of custom visitors on the resulting AST that check the document against a variety of style guidelines.

I'm not sure that the complexity of the AST is reason enough to keep it under wraps; I would argue that it's worth making it accessible and then seeing if that results in either new less-related modules and programs and/or new issues and pull requests to improve the AST. Either would be a valid indication that the parse tree is useful to developers.

@lukeapage In addition to your recommendation about recreating parse, would it be more appropriate for less.parse to return the root node or a ParseTree instance?

[seven-phases-max]

@jackwanders

I would argue that it's worth making it accessible and then seeing if that results in either new less-related modules and programs and/or new issues and pull requests to improve the AST.

The less.parse and the tree is/was accessible in v1.x (i.e. for several years) and I can't see it leaded to something valuable but certain misuses, sorry. So if killing "public" less.parse could make compiler/programmatic-rendering code more straight-forward I'm for it.

To not sound unreasonable, here's basic example:

// ..........
// a.less:

@qux: true;

.foo(@bar) {
    @baz: replace(@bar, a, div);
    @color: #123;
}

// ..........
// b.less:

@foo: a;

.body {
    .baz();
    @{baz}:hover when (@qux) {
        color: @color;
    }
}

.baz() {
    .foo(@foo);
    @import "@{foo}.less";
}

Take a look at the AST of the above code and try to imagine how much code you would need to write for a module to even simply "understand" what's happening there (before it can analyse for the coding style).

[jackwanders]

I don't disagree that less syntax can get complicated. Was the de-publicizing of less.parse a decision made in order to simplify other areas of the codebase, or was it done more as a deprecation measure because it wasn't getting much use in the wild?

[lukeapage]

I don't disagree that less syntax can get complicated. Was the de-publicizing of less.parse a decision made in order to simplify other areas of the codebase, or was it done more as a deprecation measure because it wasn't getting much use in the wild?

It was done to
a) allow us to pull apart the parser that was previously doing things a "parser" doesn't typically do
b) allow us to change the interface if needed and at some point have an async implementation of eval or one of the visitors, which was previously done in the parse method

[lukeapage]

I've merged it in, its a good refactor and I don't see any harm in exposing the parseTree (it will not be useful to most people but we do expose the parse tree for visitor plugins already)

[matthew-dean]

I can't see it leaded to something valuable but certain misuses, sorry.

@seven-phases-max I don't get this at all. What harm is it to anyone else if someone "misuses" a JavaScript file? If it's from a support / question answering perspective, we can always have a "public / supported" API, yet still expose all parts of the library so that makers / developers can create unique and unexpected extensions to less without having to fork / alter the core library. Less.js devs have no obligation to provide support for third-party plugins, and we can provide fair warning that "private" methods may change, so wouldn't that be sufficient?

@lukeapage 👍 saw you merged this in as I was typing.

[seven-phases-max]

I don't get this at all. What harm is it to anyone else if someone "misuses" a JavaScript file?

@matthew-dean What I mean is that by keeping some code there "just in case" (i.e. it does not have any specific purpose except "maybe someday someone may find some use of it") we just make unnecessary complications there (so when we'll need some similar API for something a bit more real we'll come to this with this not necessary luggage of functions/interfaces we'll had to keep at least some backward-compatibility with while as it always happens that new stuff will need different functions/interfaces). Not really a big deal (since it is already been there anyway, just a matter of keeping things simple, just one of those time-saver deveopment rules: "never make an API before you know how exactly you're going to use it" :).