Skip to content

feat: double slash comments #51

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 20, 2018
Merged

feat: double slash comments #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ not documented here are subject to change at any point.*
parser.comment({ value: 'Affirmative, Dave. I read you.' });
// → /* Affirmative, Dave. I read you. */
```

```js
parser.comment({ value: 'Affirmative, Dave. I read you.', inline: true });
// → // Affirmative, Dave. I read you.
```

Arguments:

Expand Down
5 changes: 3 additions & 2 deletions lib/comment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ class Comment extends Node {
constructor (opts) {
super(opts);
this.type = 'comment';
this.inline = opts.inline || false;
}

toString () {
return [
this.raws.before,
'/*',
this.inline ? '//' : '/*',
String(this.value),
'*/',
this.inline ? '' : '*/',
this.raws.after
].join('');
}
Expand Down
14 changes: 12 additions & 2 deletions lib/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,18 @@ module.exports = class Parser {
}

comment () {
let node = new Comment({
value: this.currToken[1].replace(/\/\*|\*\//g, ''),
let inline = false,
value = this.currToken[1].replace(/\/\*|\*\//g, ''),
node;

if (this.options.loose && value.startsWith("//")) {
value = value.substring(2);
inline = true;
}

node = new Comment({
value: value,
inline: inline,
source: {
start: {
line: this.currToken[2],
Expand Down
26 changes: 22 additions & 4 deletions lib/tokenize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ module.exports = function tokenize (input, options) {
offset = -1,
line = 1,
pos = 0,
parentCount = 0,
isURLArg = null,

code, next, quote, lines, last, content, escape, nextLine, nextOffset,
escaped, escapePos, nextChar;
Expand Down Expand Up @@ -136,6 +138,11 @@ module.exports = function tokenize (input, options) {
break;

case openParen:
parentCount++;
isURLArg = !isURLArg && parentCount === 1 &&
tokens.length > 0 &&
tokens[tokens.length - 1][0] === "word" &&
tokens[tokens.length - 1][1] === "url";
tokens.push(['(', '(',
line, pos - offset,
line, next - offset,
Expand All @@ -144,6 +151,8 @@ module.exports = function tokenize (input, options) {
break;

case closeParen:
parentCount--;
isURLArg = !isURLArg && parentCount === 1;
tokens.push([')', ')',
line, pos - offset,
line, next - offset,
Expand Down Expand Up @@ -249,10 +258,19 @@ module.exports = function tokenize (input, options) {
break;

default:
if (code === slash && css.charCodeAt(pos + 1) === asterisk) {
next = css.indexOf('*/', pos + 2) + 1;
if (next === 0) {
unclosed('comment', '*/');
if (code === slash && (css.charCodeAt(pos + 1) === asterisk || (options.loose && !isURLArg && css.charCodeAt(pos + 1) === slash))) {
const isStandardComment = css.charCodeAt(pos + 1) === asterisk;

if (isStandardComment) {
next = css.indexOf('*/', pos + 2) + 1;
if (next === 0) {
unclosed('comment', '*/');
}
}
else {
const newlinePos = css.indexOf('\n', pos + 2);

next = newlinePos !== -1 ? newlinePos - 1 : length;
}

content = css.slice(pos, next + 1);
Expand Down
119 changes: 118 additions & 1 deletion test/comment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ describe('Parser → Comment', () => {
failures;

fixtures = [
// standard comments
{
it: 'should parse comments',
test: '/*before*/ 1px /*between*/ 1px /*after*/',
Expand Down Expand Up @@ -53,6 +54,97 @@ describe('Parser → Comment', () => {
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse only one empty comment',
test: '/**/',
expected: [
{ type: 'comment', value: '', raws: { before: '' } },
]
},

// double slash comment
{
it: 'should parse double slash comments',
test: '//before\n 1px //between\n 1px //after\n',
loose: true,
expected: [
{ type: 'comment', value: 'before', inline: true },
{ type: 'number', value: '1', unit: 'px', raws: { before: '\n ' } },
{ type: 'comment', value: 'between', inline: true, raws: { before: ' ' } },
{ type: 'number', value: '1', unit: 'px', raws: { before: '\n ' } },
{ type: 'comment', value: 'after', inline: true, raws: { before: ' ' } }
]
},
{
it: 'should parse double slash comments inside functions',
test: 'rgba( 0, 55/55, 0//,.5\n )',
loose: true,
expected: [
{ type: 'func', value: 'rgba' },
{ type: 'paren', value: '(' },
{ type: 'number', value: '0', raws: { before: ' ' } },
{ type: 'comma', value: ',' },
{ type: 'number', value: '55', raws: { before: ' ' } },
{ type: 'operator', value: '/' },
{ type: 'number', value: '55' },
{ type: 'comma', value: ',' },
{ type: 'number', value: '0', raws: { before: ' ' } },
{ type: 'comment', value: ',.5' },

]
},
{
it: 'should parse double slash comments when url function have nested function',
test: 'url(var(//comment\n))',
loose: true,
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'func', value: 'var' },
{ type: 'paren', value: '(' },
{ type: 'comment', value: 'comment' },
{ type: 'paren', value: ')' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse only one double slash empty comment',
test: '//\n',
loose: true,
expected: [
{ type: 'comment', value: '', inline: true, raws: { before: '' } },
]
},
{
it: 'should parse only one double slash empty comment without newline',
test: '//',
loose: true,
expected: [
{ type: 'comment', value: '', inline: true, raws: { before: '' } },
]
},

// mixed standard and double slash comments
{
it: 'should parse mixed comments #1',
test: '/*before*/\n//between\n/*after*/',
loose: true,
expected: [
{ type: 'comment', value: 'before', inline: false, raws: { before: '' } },
{ type: 'comment', value: 'between', inline: true, raws: { before: '\n' } },
{ type: 'comment', value: 'after', inline: false, raws: { before: '\n' } },
]
},
{
it: 'should parse mixed comments #2',
test: '//before\n/*between*/\n//after',
loose: true,
expected: [
{ type: 'comment', value: 'before', inline: true, raws: { before: '' } },
{ type: 'comment', value: 'between', inline: false, raws: { before: '\n' } },
{ type: 'comment', value: 'after', inline: true, raws: { before: '\n' } },
]
},

// these tests are based on the spec rules surrounding legacy
// quotation-mark–less url notation.
Expand All @@ -61,6 +153,8 @@ describe('Parser → Comment', () => {
// doesn't start with either kind of quotation mark.
// postcss-value-parser ignores those rules and allows comments within a url()
// function if the first param starts with a space.

// standard comments
{
it: 'should not parse comments at the end of url functions with quoted first argument, lead by a space',
test: 'url( "/demo/bg.png" /*comment*/ )',
Expand Down Expand Up @@ -90,6 +184,29 @@ describe('Parser → Comment', () => {
{ type: 'word', value: ' /demo/bg.png /*comment*/ ' },
{ type: 'paren', value: ')' }
]
},

// double slash comments
{
it: 'should not parse double slash as comment in url function',
test: 'url(//bar\n http://domain.com/foo/bar //foo\n)',
loose: true,
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'operator', value: '/' },
{ type: 'operator', value: '/' },
{ type: 'word', value: 'bar' },
{ type: 'word', value: 'http' },
{ type: 'colon', value: ':' },
{ type: 'operator', value: '/' },
{ type: 'operator', value: '/' },
{ type: 'word', value: 'domain.com/foo/bar' },
{ type: 'operator', value: '/' },
{ type: 'operator', value: '/' },
{ type: 'word', value: 'foo' },
{ type: 'paren', value: ')' },
]
}
];

Expand All @@ -100,7 +217,7 @@ describe('Parser → Comment', () => {

fixtures.forEach((fixture) => {
it(fixture.it, () => {
let ast = new Parser(fixture.test).parse(),
let ast = new Parser(fixture.test, { loose: fixture.loose }).parse(),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is the only thing that stands out as questionable. the existing tests should continue to run without loose, the new tests should use loose, but they should be separate. we don't want non-loose tests running under loose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shellscape i don't change any existing comments just add flag to run own comments in loose mode as you can do in other tests with loose

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah disregard that, i'm juggling too much at the moment.

index = 0;

ast.first.walk((node) => {
Expand Down
112 changes: 111 additions & 1 deletion test/function.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,94 @@ describe('Parser → Function', () => {
]
},
{
it: 'should parse url function with quoted first argument',
it: 'should parse url function with single quotes',
test: 'url( \'/gfx/img/bg.jpg\' )',
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: ' \'/gfx/img/bg.jpg\' ' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse url function with single quotes (loose)',
test: 'url( \'/gfx/img/bg.jpg\' )',
loose: true,
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'string', value: '/gfx/img/bg.jpg', raws: { before: ' ', after: " ", quote: '\'' } },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse url function with double quotes',
test: 'url( "/gfx/img/bg.jpg" )',
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: ' "/gfx/img/bg.jpg" ' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse url function with double quotes (loose)',
test: 'url( "/gfx/img/bg.jpg" )',
loose: true,
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'string', value: '/gfx/img/bg.jpg', raws: { before: ' ', after: " ", quote: '"' } },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse absolute url function',
test: 'url( http://domain.com/gfx/img/bg.jpg )',
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: ' http://domain.com/gfx/img/bg.jpg ' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse absolute url function (loose)',
test: 'url( http://domain.com/gfx/img/bg.jpg )',
loose: true,
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: 'http' },
{ type: 'colon', value: ':' },
{ type: 'operator', value: '/' },
{ type: 'operator', value: '/' },
{ type: 'word', value: 'domain.com/gfx/img/bg.jpg' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse absolute url function with single quotes',
test: 'url( \'http://domain.com/gfx/img/bg.jpg\' )',
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: ' \'http://domain.com/gfx/img/bg.jpg\' ' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse absolute url function with double quotes',
test: 'url( "http://domain.com/gfx/img/bg.jpg" )',
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'word', value: ' "http://domain.com/gfx/img/bg.jpg" ' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse url function with quoted first argument',
test: 'url("/gfx/img/bg.jpg" hello )',
expected: [
{ type: 'func', value: 'url' },
Expand All @@ -70,6 +157,29 @@ describe('Parser → Function', () => {
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse absolute url function with quoted first argument',
test: 'url("http://domain.com/gfx/img/bg.jpg" hello )',
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'string', value: 'http://domain.com/gfx/img/bg.jpg', raws: { quote: '"' } },
{ type: 'word', value: 'hello' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse absolute url function with quoted first argument (loose)',
test: 'url("http://domain.com/gfx/img/bg.jpg" hello )',
loose: true,
expected: [
{ type: 'func', value: 'url' },
{ type: 'paren', value: '(' },
{ type: 'string', value: 'http://domain.com/gfx/img/bg.jpg', raws: { quote: '"' } },
{ type: 'word', value: 'hello' },
{ type: 'paren', value: ')' }
]
},
{
it: 'should parse rgba function',
test: 'rgba( 29, 439 , 29 )',
Expand Down