Remove brace_style = "collapse-preserve-inline" and add brace_preserve_inline new option

[Cobertos]

Added brace_preserve_inline and removed brace+style = "collapse-preserve-inline"

Adds compatibility for old collapse-preserve-inline
Changes all defaults, documentation, and README to match
Adds tests for this stuff
Code is jsHinted
Adds new option --brace-preserve-inline, -V, and new property brace_preserve_inline to opt object

[bitwiseman]

First: Wow! Way to dive in! Thanks!

Next: I'm sorry, but I'm going to have squash your dreams a bit.

1. This project is used by many many other projects, and I'm not ready to do a breaking change such as removing collapse-preserve-inline. So, I'm first going to have to ask you to put back that setting. Internally, you can keep the separate boolean you've added, but on a interface level, we can't break like that.
2. I think I'd rather see the cli option for this done a bit differently - in fact in a kinda new way. Instead of adding a new option, could you make the brace-style setting accept multiple options? In the form: [collapse|expand|end-expand|none][,preserve-inline]. So, you could have collapse,preserve-inline. (But you'll still need to keep collapse-preserve-inline for now and we'll remove it in a later version.) What do you think of this? I'm open to discussion.
3. More tests. I haven't looked deeply at what you have, but it feels a bit sparse.

Finally: Seriously, thank you for contributing.

[Cobertos]

This change still accepts "collapse-preserve-inline", it just rewrites it
to "collapse" and turns brace_preserve_inline to true which gives the same
behavior as "collapse-preserve-inline" (I just looked at how you or
whomever do compatibility for "expand-strict" and made it like that).

However that ends up being defined in the config I really had no issues
with. A comma separator would work and not hard to implement, I was just
following how the other options were implemented but it was starting to get
crowded :P.

I should probably add a test though for backward compatibility though. I'll
probably expand the tests to try both brace_preserve_inline to true and
false and that'll up the tests to 24 (not accounting for the tests that add
{}, foo = bar;, etc). I can see a couple other cases I can add tests for so
I'll do that and resubmit.

[bitwiseman]

Change still accepts collapse-preserve-inline: ah, yes, I see that now. https://github.com/beautify-web/js-beautify/pull/1058/files#diff-655cdbdfd52b6516fe4d5d2e53a7af50R314

[bitwiseman]

@Coburn37 - Just waiting for your changes.

@evocateur @einars
I think the comma delimited thing I suggested here could be a way for the project to continue to expand options without continuing to expand the number of command-line options (and run out of letters in the alphabet for them). Does this make sense? I'd appreciate your feedback.

[evocateur]

Comma-delimited "meta option" values make sense, here. Another common pattern (easier to implement with most cli arg parsers) is repeated flags that evaluate to arrays, e.g. -f foo -f bar becomes ['foo', 'bar'].

[einars]

re:repeated flags, -f foo -f bar is unambiguous and nice, and will probably be tricky to implement so that overriding the configuration file/-s works correctly and does what's expected.

Comma-separating might kick a can of worms open — I -f foo,bar works, should -f "foo, bar" work as well? Or, -f "foo bar", sounds like a logical thing to work. If comma works, why wouldn't the similar -f "foo; bar"or even-f foo+bar`?

So, I'd probably just split such options on non-[a-z0-9-_] characters.

[Cobertos]

For the two opt packages being used in the project:

• Npm's nopt
  • supports the -f foo -f bar out of the box.
  • Comma separation should be as easy as .split(',') or the regex @einars mentioned and then read (although it doesn't give the nice failure that specifying it in the params for nopt would.
• Python's getopt
  • Doesn't support -f foo -f bar or f foo,bar out of the box but I'd say the first one would look nicer in the code.

As for the configuration file, the csv list would be the easiest to do. To support multiple values of one argument, brace_style could then be a single string, an array with one string, or an array with two strings.

I'm going to go ahead with the comma separation for now, if that needs to change so be it. I would rather not change the type of the config files property.

[bitwiseman]

@Coburn37 - There isn't strong opinion either way, and you're the one writing it, so it is your call whether you want to do commas or multiple args. The worst case is it gets changed later. 😄

[Cobertos]

So now there's no brace_preserve_inline and brace_style can be used with [collapse, expand, end-expand, none] and also with an optional preserve-line. It's separated by [a-zA-Z0-9_-]+ but all the docs say to use ,.

Anyway, the build says failing but it looks like a Python SSL error that I have no way of fixing (and one of the Travis CI subbuilds actually passed if you look in there) so someone will need to retrigger the builds.

[Cobertos]

Anyone have any input on whether the order of the csv values in brace_style should matter or not before this is merged? Currently they do.

[bitwiseman]

As we get more csv parameters, this will need to change. Fine for now.

[bitwiseman]

@Coburn37 - They should processed in order and if the settings conflict the last one should win.

The following two should have the same result:

"collapse,expand,preserve-inline,collapse" 
"collapse,preserve-inline" 

However, that is something that can be done in another PR (which I really hope you'll do). This PR has gone on long enough. You've done great work. I'm merging and we can iterate further changes.