1018586 - Pretty Print should indent switch statement like CodeMirror and MDN Coding Style

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Adrian Aichner [:anaran]]

Pretty Print currently does:

switch (x) {
case a:
  foo();
  break;
default:
  bar()
}

CodeMirror Shift-Tab and
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Control_Structures
do and say respectively:

switch (x) {
  case a:
    foo();
    break;
  default:
    bar()
}

I am separating my implementation from Bug 975477 comment 5 now in the hope they will be more acceptable separately.

Comment 1

[Adrian Aichner [:anaran]]

Attachment: 0001-Bug-1018586-Pretty-Print-should-indent-switch-statem.patch

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

We don't use the mdn coding style in devtools, but I'll take a look at the code and if it doesn't complicate things too much I'll r+.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8432125 [details] [diff] [review]
0001-Bug-1018586-Pretty-Print-should-indent-switch-statem.patch

Review of attachment 8432125 [details] [diff] [review]:
-----------------------------------------------------------------

Can't see enough context, when you upload patches make sure you format them with 8 lines of context. See the -U 8 stuff here: https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in

However, in this special case where pretty-fast is an upstream project, can you just submit PRs there and then I can just merge upstream when PRs land.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Needs test(s) as well.

Comment 5

[Adrian Aichner [:anaran]]

OK, I'll submit a PR with test.

Comment 6

[Adrian Aichner [:anaran]]

(In reply to Nick Fitzgerald [:fitzgen] from comment #4)
> Needs test(s) as well.

Oh wait, there is an existing switch indentation test that will fail with my patch.
Shouldn't I just update that and be done with the test coverage?

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Sounds good.

Comment 8

[Adrian Aichner [:anaran]]

Attachment: Upstream pull request, with updated test coveralge

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8432726 [details] [review]
Upstream pull request, with updated test coveralge

Created bug 1020587 to update pretty-fast in the tree.

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Resolved as part of https://hg.mozilla.org/mozilla-central/rev/e157171a4df1