Incorrect indentation after case statement

[adamreisnz]

Not sure if this is the right repo or if the language deals with indentation rules, but consider the following:

switch (foo) {
  case 'bar':
  //cursor here after enter on the above line
    //expect indentation to be here, 
    //especially if there are already lines of code indented at this level          
}

Pressing enter after the case 'bar:' statement indents the next line at the same level as case. I would expect it to increase indentation by one unit.

[willxy]

I would too. A google search led me here, as well as to earlier tickets, where it looks like the indent was added Jun '24 (#36) then issues reported Aug '14 (#49) then the indent taken out Apr '15 (21fae9f).

[aryzing]

^ Same.

[adrianlafond]

I agree with you guys, but I've looked at the javascript language-settings file and I'm a bit stumped as to how to fix it myself. Anyway, I think this goes back to Crockford (aaargh!) who advocated using 4 spaces for indents which led to very deep indents under case statements and thus his recommendation (which became virtual dogma) that code under case statements not be indented. But nowadays most JavaScript devs use 2 space indents (as in @adambuczynski 's example above) rendering the issue moot. Long past time to add that indent back in. But in the meantime, anybody know how to hack Atom to fix it manually?

[Alhadis]

But nowadays most JavaScript devs use 2 space indents

This is why I hate people.

[adamreisnz]

This is why I hate people.

I used to be a hater. But I learned to love the 2 space indent and embraced it in all it's glory 😄 🎉

Edit: full disclosure, I still use the tab key, but it gets converted to 2 spaces in code automagically.

[Alhadis]

Must... not... derail thread...

[droberts-sea]

Looks like this is addressed in #372

[droberts-sea]

I don't know if anyone is watching this thread, but this is the one I could find on this issue that is still open. Currently this behavior is disabled because "it wouldn't know when to outdent":

language-javascript/spec/javascript-spec.coffee

Line 1726 in 55af017

it "doesn't indent case statements, because it wouldn't know when to outdent", ->

I think this is an oversight - it should be possible to tell when to outdent. You should outdent when you see another case xyz:, a default:, or a closing curly brace (double outdent, since the switch is also over).

I would love to see this as at least an opt-in feature. Doesn't have to be the default, but I don't see any theoretical reason it shouldn't be possible.

[shanehughes3]

it wouldn't know when to outdent

Then how do most other text editors do it? It's by no means an impossible task. By most style guides this default indentation is wrong. See google, airbnb, standard.

[chfritz]

I agree that this is not an impossible task, but it is not trivial:

You should outdent when you see another case xyz:, a default:

That is actually not correct. The issue is that if you don't use break then the scope continues on into the next case statement:

> switch (1) { case 1: console.log(1); case 2: console.log(2) }
1
2

Indenting the above example as:

switch (1) {
  case 1:
    console.log(1);
  case 2:
    console.log(2);
}

would actually mislead the user into thinking that the scope of the first case statement ends when the case 2 term is outdented.

Now, practically speaking, most people will follow the recommended practice of always using a break, but that begs the question of whether or not the editor is supposed to help the user to do things right, by indenting as-is, or should just makes things look pretty assuming that the user already is doing things right, i.e., indent as-should-be.

I'm in the camp of the former, so I would actually aim for a logic where we do indent if there is a break statement in the case, but don't if there isn't.

BTW, none of this is currently possibly in atom.io given that the indentation is inductive (just from the previously line to the current without look-ahead or anything). But in the new tree-sitter based logic this will be possible, so this is a good time to discuss this.

[shanehughes3]

I would disagree with the idea that the editor should help the user do things right through indentation in this case. While indentation does help with flow interpretation, here it would be the job of a linter, not indentation. For example, what if a user's (ill-advised) intention with the following block is to execute block A and B on case 1, but only block B on case 2?

switch (i) {
    case 1:
        // block A
    case 2:
        // block B
        break;
}

In this situation the indentation makes perfect sense - it would be up to a linter to advise the user on the poor design. Further indenting case 2: makes it unclear that the value 2 provides an entry point into this statement that will only execute block B without any side effects from block A.

BTW, none of this is currently possibly in atom.io given that the indentation is inductive (just from the previously line to the current without look-ahead or anything).

This I was not aware of, and is very interesting. I do hope that with new indentation logic, this issue might be revisited.

[chfritz]

Just in terms of soliciting input, do you think the following would make sense?

switch (i) {
    case 1:
    // block A
    case 2:
        // block B
        break;
}

The logic being that block A does not end at case 2: but falls-through.

I'm really not clear what the right thing to do is, so discovering all perspectives seems important.

We are actually revisiting this in the new logic -- that's why I'm chiming in here, because I'm trying to contribute a logic that works for everyone (to the degree possible, given that some of it is seems to be a matter of opinion). You can get a sense of where my mind is at by using my sane-indentation package, which is where I'm developing a new logic I want to propose.

[shanehughes3]

Personally, there are two perspectives I've seen on switch indentation, the only difference being in the indentation level of the case lines:

// type A:
switch (i) {
case 1:
    // code
case 2:
    // code
    break;
default:
    // etc.
}

// type B:
switch (i) {
    case 1:
        // code
    case 2:
        // code
        break;
    default:
        // etc.
}

Type B is all I ever see in javascript - type A is found in the linux kernel style guide and other older guides. I've grow accustomed to type B since I've been writing mostly js lately. I do think that all case statements should be at the same indentation level, and all code within all cases should be at the same level. I think the situation that you are thinking of correcting - the example I presented - is usually considered bad practice, so I don't think that it really needs to be addressed (except for in a linter). So that would mean that a break statement has no impact on indentation. But I'm just a sample size of one.

Thank you for your work on these packages! I just installed sane-indentation and will check it out.

[adamreisnz]

My two cents would be to have this be the default indentation:

switch (i) {
  case 1:
    //code
  case 2:
    //fall through code
    break;
  case 3:
    //more code
  default:
    //etc.
}

Simple, clean and concise.
I also agree that all case statements should be at the same indentation.