Add the ability to manually format Javascript code (#87)

[sbraconnier]

Default format shortcut is Ctrl+Shift+F (like Eclipse). I hesitated between Eclipse (Ctrl+Shift+F), Intellij (Ctrl+Alt+L) and Postman (Ctrl+B).

Note that I also add the menu item "toggle comment" option to the new contextual "code" menu.

[tonygermano]

These changes all look good to me. I split the original commit into two because while adding the ability to toggle comments via the context menu makes sense, it is not related to the other change of being able to format the code. I also expanded the commit messages to better describe the changes.

As an example of what the code formatting can do, here is a before:

And after:

The context menu changes after both commits appears as the following:

[pacmano1]

In the context of QA on the compiled running behavior:

• Appears to format e4x code
• Appears to not break things like XML literals (e.g. it might indent a var x = <someliteral/> but it will not indent the subsequent line (which it should not) should the XML blob continue
• Note sure the right click toggle comment is needed, but not a breaker.

One issue - it should probably not format bad code (i.e. the editor window should be clean prior to format)

[gibson9583]

@pacmano1 - noted same issue on malformed JS. Only option around this would be to call validation first, but I don't think it's a deal breaker after kicking around with SC. Some beautifiers will and some won't format "bad" code.

[tonygermano]

In the context of QA on the compiled running behavior:

• Appears to format e4x code
• Appears to not break things like XML literals (e.g. it might indent a var x = <someliteral/> but it will not indent the subsequent line (which it should not) should the XML blob continue
• Note sure the right click toggle comment is needed, but not a breaker.

One issue - it should probably not format bad code (i.e. the editor window should be clean prior to format)

I think the right-click toggle comment is nice to have for users who forget the hotkey combination or didn't even know the option existed.

I disagree about it not formatting bad code. If I copy code that is a mess and ask it to format, I expect it to do its best. I don't need it complaining to me that the code is not perfect, because I might be asking it to format so that I can more easily find mismatched or misplaced parentheses or braces.

[pacmano1]

well, I'm sure it's predictable in an algorithmic sense, it just feels unpredictable from a UI perspective. I'd probably warn and allow forever dismiss.

If everyone else is OK with this behavior, I can agree to disagree.

[tonygermano]

@pacmano1 The engine still won't let you save the channel with code that doesn't validate.

I just typed this mess into vscode

var     a           =1
   var  b=    2;

   for(  vara i of [1,2,3])) {
    const c=4
   }

   function d() {
    while (true)
        false;}
   }

And it formatted it to this without complaining, even though there are several syntax errors.

var a = 1
var b = 2;

for (vara i of[1, 2, 3])) {
    const c = 4
}

function d() {
    while (true)
        false;
}
   }

I don't think it's the job of the formatter to warn you about invalid code. It just does its best to tidy up. There are other mechanisms that already exist to tell you that your code has errors, and formatted code can make it easier to see where you made a mistake.

[pacmano1]

I came across this same issue with the content formatter. A different PR. For example, both built in prettyPrints will format the "wrong" data type.

So if I'm understanding you correctly, your position is the developer should know better.

[tonygermano]

I'm not sure what you mean by the developer should know better. I think the formatter should do what it is asked to do, and there is no implication that because it improves the readability of the code that the code is syntactically correct.

The program will still alert the user if there is a syntax error whether the format feature is used or not. If you want to test if the code has syntax errors, there is already a validate button for that.

[pacmano1]

I said my peace here, approving. I will remind you all of this in the future with the map variable formatter that does have a similar "problem". Cool feature though @sbraconnier.

[pacmano1]

Re-approving - previous comments noted.

[mgaffigan]

Notes:

• Formatting fails pretty hard with var x = <{a} {a}={a} />;
• The user caret is moved to the start of the formatted selection (rather than wherever it was prior to formatting)
• The editor in general has some odd behavior with characters outside of the BMP

I don't think any of those are cause for correction. LGTM