Skip to content

Conversation

isqua
Copy link
Contributor

@isqua isqua commented Sep 27, 2015

Discussion: #6
In .csscomb.json:

{
    "syntax": {
        ".css": "scss",
        ".lss": "less"
    }
}

If the implementation is ok, I’ll write tests.

@tonyganch
Copy link
Member

Here: https://github.com/csscomb/core/blob/dev/src/core.js#L135
You want to use _extractSyntax in this place too.

@tonyganch
Copy link
Member

If the implementation is ok

It is. Thank you 🐘

@isqua
Copy link
Contributor Author

isqua commented Sep 27, 2015

🆙

test/core.js Outdated
Copy link
Member

Choose a reason for hiding this comment

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

You can't be sure that map was copied because {} is the default value it gets even if configuration fails.
I'd offer replacing on line 27 syntax: {} with syntax: {'.css': 'scss'}, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assert.equal check pointers, not values:

assert.equal({}, {}); // => false

let a = {};
let b = a;
assert.equal(a, b); // true

...

Copy link
Member

Choose a reason for hiding this comment

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

Good to know, thank you!

@isqua
Copy link
Contributor Author

isqua commented Sep 27, 2015

Is Map usage ok? If so, I’ll squash commits.

@tonyganch
Copy link
Member

Map is ok. Let me know when to merge.

@isqua
Copy link
Contributor Author

isqua commented Sep 27, 2015

It’s ready to merge now 🔥

@tonyganch tonyganch merged commit 4f18208 into csscomb:dev Sep 27, 2015
@tonyganch
Copy link
Member

Thank you very much 👯

@isqua isqua deleted the issues/6 branch September 28, 2015 15:16
@isqua
Copy link
Contributor Author

isqua commented Sep 28, 2015

When csscomb.js will support this feature?

@rodfersou
Copy link

I'm interested in this feature too

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.

3 participants