Skip to content
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.

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

Less Beautify Crashing when using multiple parameters in mixin #61

Closed
sebastiankade opened this issue Aug 13, 2014 · 10 comments
Closed
Assignees

Comments

@sebastiankade
Copy link

Hi,

Love the package.

I'm getting:

Uncaught TypeError: Cannot call method 'replace' of undefined 
/atom-beautify/node_modules/prettydiff/prettydiff.js:947

for mixins with multiple parameters:

// Breaks
.mixin(@variable, @anotherVariable) {
}

// Ok
.mixin(@variable) {
}

Cheers

@Glavin001 Glavin001 added the bug label Aug 13, 2014
@Glavin001 Glavin001 self-assigned this Aug 13, 2014
@Glavin001
Copy link
Owner

Sounds like a similar issue as is happening with PHP. See #51 (comment)

I am going to have to test these out tomorrow. Thank you for letting me know!

@Glavin001
Copy link
Owner

I believe your source code should be:

// Breaks
//.mixin(@variable, @anotherVariable) {
//}

// No longer breaks
// Use a semicolon instead of comma
.mixin(@variable; @anotherVariable) {
}

// Ok
.mixin(@variable) {
}

See http://lesscss.org/features/#mixins-parametric-feature-mixins-with-multiple-parameters

@Glavin001
Copy link
Owner

// Breaks
.mixin(@variable; @anotherVariable) {
}
// Ok
.mixin(@variable) {
}

Correctly beautifies to:

// Breaks
.mixin(@variable; @anotherVariable) {

}
// Ok
.mixin(@variable) {

}

Thanks for using Atom Beautify! Let me know if there's anything else :).

@Glavin001 Glavin001 changed the title Less Beautify Crashing Less Beautify Crashing when using multiple parameters in mixin Aug 13, 2014
@Glavin001
Copy link
Owner

For future reference, Atom Beautify uses PrettyDiff for LESS and SASS beautification. You can test out your source code beautification here: http://prettydiff.com/?m=beautify (remember to show advanced options and select the CSS language)

Errors in beautification may be an issue with PrettyDiff - we used JSBeautify before and that did not work well in all cases for LESS so we switched to PrettyDiff which was recommended. If it turns out to be an issue with PrettyDiff (this time it was just a user error, but next time perhaps) then please create an issue there and let me know of it's progress :). Thanks!

@sebastiankade
Copy link
Author

Damn that sucks, our whole LESS base is using comma separated and most LESS libraries used as well. Its funny the LESS homepage has the main example with a comma

As they are meant to be supported for either comma or semicolon:
"Parameters are either semicolon or comma separated. It is recommended to use semicolon."

I'll chase it up with PrettyDiff.

Cheers

@Glavin001
Copy link
Owner

In that case it sounds like a PrettyDiff bug. Could you take a look at their issues and make one if applicable? It may already be in the works.

@Glavin001 Glavin001 reopened this Aug 13, 2014
@Glavin001
Copy link
Owner

Let me know if there's anything I can do on my end -- such as update versions or add a new configuration option, etc.

@myEdOnline
Copy link

Will do.

Thanks

@Glavin001
Copy link
Owner

I created an issue here: prettydiff/prettydiff#23

I am unfamiliar with the Pretty-Diff source code. Hopefully a fix is not too difficult.

@myEdOnline
Copy link

@Glavin001, @austincheney just fixed the issue in Pretty-Diff (prettydiff/prettydiff#23 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants