Indentation of `case` differs between typescript and tsserver's format about typescript.el HOT 9 CLOSED

Not sure on the status on this, but I have a rough patch that I use to get the something similar to the tsserver behaviour. I have basically just copied the relevant part from js-mode. I don't think my current solution is particularly well tested, but if there is desire then I'm happy to try and turn this into a PR.

By default my solution should retain the existing behaviour of typescript-mode (which I think was a constraint). You can then set the tsserver-like behaviour using:

(setq typescript-switch-indent-offset typescript-indent-level)

Anyways, here is a link to the commit: jack-williams@c01dd01

One problem with it is that it will indent comments like:

export function foo(x: "x" | "y"): string {
    switch (x) {
        case "x": return "hello";
        case "y": return "world";

            // should never hit this
        default:
            return "never";
    }
}

but tsserver will format them like:

export function foo(x: "x" | "y"): string {
    switch (x) {
        case "x": return "hello";
        case "y": return "world";

        // should never hit this <-- no offset
        default:
            return "never";
    }
}

I believe this behaviour is inherited from js-mode.

from typescript.el.

From what I can tell, these lines here seem to be relevant to this behaviour:

https://github.com/ananthakumaran/typescript.el/blob/master/typescript-mode.el#L1740-L1741

Basically they cancel out the case-statement's "natural" indentation. I tried, but just stripping these lines out/disabling them alone doesn't seem to be much use.

It seems we'll also need a way to inform the parser that it's currently processing a case-statement to ensure the body of the case-statement in the following lines also gets an extra level of indentation.

from typescript.el.

I would like to fix it also. The best place is to look at js--proper-indentation and see what they do. typescript-mode is a fork of js-mode.

from typescript.el.

Agreed it should be fixed.

And your idea is a good one. Unfortunately I only have time for cursory checks here and there these days.

I can assist, but I doubt will be the one who writes the patch.

from typescript.el.

Hmm.... I'm not a fan of the way tsserver does it. I prefer this:

switch (something) {
case a:
  ...
case b:
  ...
}

to what I understand tsserver does:

switch (something) {
  case a:
    ...
  case b:
    ...
}

This does not seem to be configurable in tsserver. From what I see it is possible to switch between indentation styles None, Block and Smart. If I'm reading the test files correctly, none of them would produce the result that typescript-mode produces (and that I prefer)... ☹️

I'm commenting on this now because I ran into these bad cases today:

export enum Something {
CASE,
DEFAULT,
  MOO,
}

const q = {
case: "b",
default: "a",
  moo: "blah",
}

(Oh, and in the 2nd case, the fontification is also off.)

I figure I'd fix that but then I saw the comment in the indentation test file about how the mode is not doing switch indentation right and the found this issue.

For the record js-mode and js2-mode do the switch indentation like typescript-mode currently does. And yes, js-mode has a fix for the indentation of const q =... (but not for the buggy fontification). js2-mode indents and fontifies const q =... correctly. (But as I mentioned in another issue js2-mode performs fontification while parsing a syntax tree out of the buffer. Not easily adoptable for typescript-mode.)

I'm not sure where to go from here. Fixing the switch indentation to conform to tsserver would pull the rug from under the feet of those who prefer what typescript-mode currently does.

from typescript.el.

I'm not a fan of the way tsserver does it. I prefer this

I'm not sure if the same applies to the Typescript-community at wide yet, but in the Go-community there's been a consensus on that whatever gofmt makes the code look like is the Go coding-style.

No exceptions. None-compliant code does not lint and is not merged. It's not very flexible (or Emacsy to put it into context), but on the flip-side it makes things very simple.

I've pretty much seen tsserver-based formatting take the same role in the (admittedly limited number of) Typescript-projects I've been involved with. I guess I just assumed this was a wider trend than it may actually be.

I'll be honest and admit that in your example I personally very much prefer the way tsserver does things. It seems more natural/logical to me: It's delimited by a block-delimiter. Ofcourse it should have block-level indentation.

Putting personal opinions aside though, I'm pretty sure you will see most other editors with Typescript-support strive to adhere to the tsserver "standard" (or quasi-standard if you like).

This does not seem to be configurable in tsserver. From what I see it is possible to switch between indentation styles None, Block and Smart.

Which brings me to my next question: Do we (or anyone really) have any idea of how widely used these options are?

I know that I almost never customize my tsconfig.json apart from aspects affecting actual compilation. But again: that's not hard data. It's just me assuming things based on my own habits and preferences.

I'm not sure where to go from here. Fixing the switch indentation to conform to tsserver would pull the rug from under the feet of those who prefer what typescript-mode currently does.

Taken to the extreme, this really attitude applies to pretty much any shipped code, even bugs. Ref XKCD.

I guess what this all boils down to is a core question about what we should strive for:

• should we strive for a very Emacsy mode which is everything to everyone, where most aspects can be customized (without the code turning into a nightmare-ish state-machine which is impossible to debug or reason about)
• or should we try to go where (I believe) the community is going, by conforming to a less flexible Typescript standard-formatting?

I'm not the guy to make that decision, but I think it may be worthwhile to have a discussion where we settle this properly instead of spending energy cautiously moving in a undecided direction whenever issues like these pop up.

With that said: Wherever we decide to go, I think one high priority must be not to alienate whatever community and user-base we have built. And I'm willing to do a lot of compromising on my personal preferences to make that happen :)

from typescript.el.

ping @josteink @lddubeau

from typescript.el.

Right. Sorry about the absence. And thanks, @jack-williams, for providing an actual patch.

So let me try to quickly recap the situation as I see it:

Before this patch the status was that:

• there does not seem to be a universal agreement among the developers/users how typescript-mode should indent code.
• if we agree on how things should be, we may still lack the man-hours to implement it (since indentation-code easily is our most complex piece of code)
• either way, discussion on those matters didn't really take us anywhere... actionable.

With this patch:

• typescript-mode now adheres more closely to the way tsserver does things (and personally, I'm very happy about that).
• There's a single regression with regard to comments inside switch/case-statements.

If that's all, I think that's an improvement and I think you should submit a PR for it. If anyone has any objection to the fixes, vs the regressions, that will be the place for them to do so.

from typescript.el.

No problem; sorry for the ping but I wasn't sure if notifications were switched off for this thread.

I'll put in PR shortly!

Indentation can be pretty hairy so I'm happy waiting a while before merging, letting people it out. I might need some help with the tests but I'll give it a go first.

from typescript.el.