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

braces "expand-strict", return { } should be on same line #236

Closed
mitermayer opened this issue Apr 16, 2013 · 9 comments · Fixed by #240
Closed

braces "expand-strict", return { } should be on same line #236

mitermayer opened this issue Apr 16, 2013 · 9 comments · Fixed by #240

Comments

@mitermayer
Copy link

when running with settings expand-strict it should still allow

return {
    foo: val
}

instead of

return 
{
    foo: val
}

to avoid javascript ghost semi collon auto add

@evocateur
Copy link
Contributor

This is why you shouldn't use expand-strict in JavaScript.

@bitwiseman
Copy link
Member

@mitermayer, I think you want expand instead. It will produce output that does not have this problem.

@einars, Why do we even have the expand-strict option?
It was added as part of cdee74c, but I don't see any comment as to why.

We know it will produce javascript that is functionally different from the input in cases like this one. Perhaps we should remove it. If not, we should at least add to the help output warning about this known issue.

@einars
Copy link
Contributor

einars commented Apr 16, 2013

I've dug up the corresponding correspondence with author, I quote him,

Enforced braces expanding works in cases, when you define an object.
For example, if we have

a = {field1: value1, field2: value2};

then styling with the option "Braces on own line" does not really work
as implied, because the result will be

a = {
 field1: value1,
 field2: value2
};

So, I added the option which tolds to the script that it should produce:

a =
{
 field1: value1,
 field2: value2
};

Personally, I like when paired braces are in the same columns and
can be easily spotted as a solid context. I don't think it should be
turned on by default.

Since then I made more changes, to name a few - fixed an error in my
'case' formatting code, added php operators, so that I can now style
not only javascript but php codes as well.

I think it's time to let go of this strange and subtly destructive option.

@mitermayer
Copy link
Author

The only reson why i used expand-strict was because it allowed inline

Function(){}

Is there any other way to achieve that with the expand option ?

@mitermayer
Copy link
Author

Thats only for anonymous empty functions

@bitwiseman
Copy link
Member

That's even more awesome - the setting that will break return {a:1} by forcing the curly brace to the next line, is also the setting that allows Function(){} by not forcing empty braces to the next line. face + 🌴.

We should fix that.

@mitermayer
Copy link
Author

But we should have a setting to allow

Var b = cb || function(){}

Instead of appearing

Var b = cb || function()
{}

So what are other options ?

@bitwiseman
Copy link
Member

We could make empty braces always collapse.

@mitermayer
Copy link
Author

@bitwiseman i think that would be a great idea, could we create an issue to address that ?

bitwiseman added a commit to bitwiseman/js-beautify that referenced this issue Apr 18, 2013
While removing expand-strict, found that brace tests were uneven.
Made all tests appear for all three settings, and adjusted for each.
There are still some minor odd behaviors around empty braces, but they are
non-breaking.

Closes beautifier#236, beautifier#237
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 a pull request may close this issue.

4 participants