Skip to content

Fix AST to include text for single line comments#2858

Merged
seven-phases-max merged 1 commit intoless:masterfrom
zzzzBov:patch-1
Mar 31, 2016
Merged

Fix AST to include text for single line comments#2858
seven-phases-max merged 1 commit intoless:masterfrom
zzzzBov:patch-1

Conversation

@zzzzBov
Copy link
Copy Markdown
Contributor

@zzzzBov zzzzBov commented Mar 31, 2016

The AST didn't include the contents of single line comments because of a tyop in a property name.

I used

function lessAST(filename, options) {
    options = options || {};
    options.filename = path.resolve(filename);
    return new Promise(function (res, rej) {
        fs.readFile(filename, 'utf8', function (err, data) {
            if (err) {
                rej(err);
            } else {
                res(data);
            }
        });
    }).then(function (data) {
        return new Promise(function (res, rej) {
            less.parse(data, options, function (err, tree) {
                if (err) {
                    rej(err);
                } else {
                    res(tree);
                }
            });
        });
    });
}
lessAST(filename).then(function (tree) {
  fs.writeFile('tmp.json', JSON.stringify(tree, null, 2));
});

to view the AST locally. I haven't yet written any tests for this particular case.

@zzzzBov
Copy link
Copy Markdown
Contributor Author

zzzzBov commented Mar 31, 2016

Here's a quick little test I threw together:

less.parse('//Hello World!\n.foo{bar:baz;}', {filename: ''}, function (err, tree) {
    if (err) {
        console.error(err);
    } else {
        console.assert(tree.rules[0].value === '//Hello World!', 'The single line comment should have a value of "//Hello World!".');
    }
});

@seven-phases-max seven-phases-max merged commit f4957bd into less:master Mar 31, 2016
@seven-phases-max
Copy link
Copy Markdown
Member

Thanks!

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.

2 participants