Bring back space_after_anon_function

[gabrielmaldi]

This PR adds the option jslint_happy_align_switch_case which allows to opt out of switch-case alignment when using jslint_happy.

With jslint_happy_align_switch_case = true (the default):

switch (something) {
case 1:
  break;
case 2:
  break;
}

With jslint_happy_align_switch_case = false:

switch (something) {
  case 1:
    break;
  case 2:
    break;
}

Most changes in this PR are trimmed trailing whitespace, use ?w=1 to focus on the real changes.
#15, #213, #372

ToDo:

• JavaScript
• Python
• CLI
• Tests
• Docs

[bitwiseman]

Interesting. The only reason we have jslint_happy is to make jslint not complain. Why not just use? jslint_happy = false? You want to make this change so that you get function () without the odd case indents?

[gabrielmaldi]

Yes, as @rossipedia and @roman01la I just want the space between function and the parens.

I gave it some though before and came up with these alternatives for the API:

1. Remove jslint_happy in favor of two specific options for switch-case alignment and the space in the function definition.
   • This has the nice property that is future-proof, because with good defaults you can always add new options to make jslint happy
   • I didn't go this way because it is a breaking change and because I saw that you used to have a space_after_anon_function that was deprecated
2. Add a specific option for the space in the function definition
3. Add a specific option for the switch-case alignment

Options 2 and 3 have the downside that if in the future jslint_happy makes more changes, we'd have to add more options to deal with that.

So I ended up going with option 3 because it seemed the least disruptive. What I'd like is actually option 1 and to also keep jslint_happy as a switch to turn some specific options on; but this decision obviously is beyond me 😃

[bitwiseman]

I went ahead and cleaned up the spaces in master. Thanks for pointing them out.

There is a long standing tension between added formatting options and keeping the tool from degenerating into a completely configurable but untestable hell. In #221, we've discussed "The beautifier basic formatter, not a general-purpose custom formatter."

It looks like what you really want add the space_after_anon_function back, but you're trying to respect the direction of the design. Thanks for trying, but you might as well just put the old setting back in - it was clearer about what it was doing. 😄

In fact, option 4 (option 1 but keep jslint_happy) sounds like the right way to go. 😄 Give that try.

[gabrielmaldi]

Thanks, I'll follow that discussion.

I pushed some changes to bring space_after_anon_function back. Go ahead and tell me if you think that something should be different (or just push minor changes yourself if you feel like it).

Just FYI, I use Sublime Text 3 with these options 😃:

{
  "trim_trailing_white_space_on_save": true,
  "ensure_newline_at_eof_on_save": true
}

[evocateur]

"trim_trailing_white_space_on_save": true (regardless of how much I agree with it) is a very noisy setting for projects that you don't own. I'd recommend disabling it, since all it does is create confusion when evaluating pull requests.

[gabrielmaldi]

@evocateur you're right, sorry.

[evocateur]

@gabrielmaldi Any news on the Python implementation? I appreciate your responsiveness to the feedback here.

[gabrielmaldi]

I mistakenly thought I had asked for some help because I had never written a line of Python before. Anyway, I dived into it and it was pretty simple since all I had to do was copy the whole structure of an existing option.

I also added the new option to the CLI and docs.

Just shoot any feedback 😃

[gabrielmaldi]

Hey, just a friendly reminder that if you guys want me to work further on this PR just go ahead and tell me what you think I should do/change 😄

[bitwiseman]

Instead, I'd suggest adding this at line 279:

// force opt.space_after_anon_function to true if opt.jslint_happy
if(opt.jslint_happy) {
    opt.space_after_anon_function = true;
}

Then below you can check for just opt.space_after_anon_function instead of two different options.

[gabrielmaldi]

Most likely I misunderstood, but we need this line here; otherwise space_after_anon_function won't be set to the desired value.

[bitwiseman]

The change you made looks like what I intended. 😄

[bitwiseman]

Sorry for the delay. Thanks again for you following through on this change, and for doing the python work as well. It looks really good.

A few minor changes (and mirror them in python) and we can merge this.

[gabrielmaldi]

I can't get this to work; it's what's making tests fail. Help? 😄

[gabrielmaldi]

Thanks for your feedback! I went ahead and made the changes you suggested. If I missed anything just tell me.

I'm having trouble getting to force space_after_anon_function to true if jslint_happy in Python. Any help will be appreciated.

[bitwiseman]

Ah, you found a bug in the python code. I've just pushed a fix. Rebase your changes on top of master and squash to one commit. Tests should then pass.

[gabrielmaldi]

My first thought was that it was probably just what you fixed in c67a9e7. But I couldn't be sure because of my little knowledge of Python 😄