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

space-before-conditional: false doesn't work on switch case statement(beautify.js) #1858

Closed
bibekoli opened this issue Nov 26, 2020 · 3 comments · Fixed by #1881
Closed

space-before-conditional: false doesn't work on switch case statement(beautify.js) #1858

bibekoli opened this issue Nov 26, 2020 · 3 comments · Fixed by #1881

Comments

@bibekoli
Copy link

bibekoli commented Nov 26, 2020

Description

It is not removing spaces between switch and () to make it like this: switch() but it is not removing space, so it becomes switch ()

Input

The code looked like this before beautification:

switch (a) {
case 1:
alert(1);
break;
default:
console.log(1)
}

Expected Output

The code should have looked like this after beautification:

switch(a) {
  case 1:
    alert(1);
    break;
  default:
    console.log(1)
}

Actual Output

The code actually looked like this after beautification:

switch (a) {
  case 1:
    alert(1);
    break;
  default:
    console.log(1)
}

Steps to reproduce

Add this option..
{
"space_before_conditional": false
}

Environment

OS: Chrome Browser Android

Settings

Example:

{
    "indent_size": 4,
    "indent_char": " ",
    "indent_level": 0,
    "indent_with_tabs": false,
    "preserve_newlines": true,
    "max_preserve_newlines": 10,
    "jslint_happy": false,
    "space_after_anon_function": false,
    "brace_style": "collapse,preserve-inline",
    "keep_array_indentation": false,
    "keep_function_indentation": false,
    "space_before_conditional": false,
    "break_chained_methods": false,
    "eval_code": false,
    "unescape_strings": false,
    "wrap_line_length": 0
}
@bitwiseman
Copy link
Member

bitwiseman commented Dec 1, 2020

I wonder whether existing users think of switch (a) as conditional. if and while have conditionals, switch has an input value which the case statements branch on. This functionality should be added as a separate option.

@sovoid
Copy link

sovoid commented Jan 13, 2021

I am new to this project and would love to contribute to this. I understand maintainers have limited time but would really appreciate any guidance on how to get started on this issue

@bitwiseman
Copy link
Member

Having more specific questions would be helpful.

In javascript implementation adding switch to this branch is a good place to start.

https://github.com/beautify-web/js-beautify/blob/854ce35867fd0ac178482a8c6b30b5bcde45fe27/js/src/javascript/beautifier.js#L557-L559

There might be other places where it is needed, but that is the main one. You'll need to update/add tests and do the python implementation as well.

raobhavya92 added a commit to raobhavya92/js-beautify that referenced this issue Jan 25, 2021
@raobhavya92
Copy link
Contributor

Hi @bitwiseman, I gave this issue a shot.
Raised PR: #1881

Please review and do let me know if there is any feedback or corrections. It's my first time contributing! :)

bitwiseman added a commit that referenced this issue Jan 25, 2021
Issue #1858- Space-before-conditional: false doesn't work on switch case statement
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants