typescript.el icon indicating copy to clipboard operation
typescript.el copied to clipboard

Inconsistent Indentation Between Multi-line Arrow Function parameters and function parameters

Open stieg opened this issue 7 years ago • 6 comments

Below are examples of 3 types of functions with single line parameters (S suffix) or double line parameters (D suffix). One would expect the behavior of single and multi-line parameters to behave the same between arrow functions and traditional named (or anonymous) functions but that does not appear to be the case. Is something possibly wrong with my settings?

Typescript-mode: v0.2

function fooS(a: string, b: string) {
  return a + b;
}

function fooD(a: string,
              b: string) {
  return a + b;
}

const barS = function(a: string, b: string) {
  return a + b;
};

const barD = function (a: string,
                       b: string) {
  return a + b;
};

const bazS = (a: string, b: string) => {
  return a + b;
};

const bazD = (a: string,
              b: string) => {
                return a + b;
              };

stieg avatar May 30 '18 22:05 stieg

Also of note... similar inconsistent behavior is noticed with a return type is added. Here is the above example with a return type set that further creates inconsistencies:

function fooS(a: string, b: string): string {
  return a + b;
}

function fooD(a: string,
              b: string): string {
                return a + b;
              }

const barS = function(a: string, b: string): string {
  return a + b;
};

const barD = function(a: string,
                      b: string): string {
                        return a + b;
                      };

const bazS = (a: string, b: string): string => {
  return a + b;
};

const bazD = (a: string,
              b: string): string => {
                return a + b;
              };

Notice how both fooD and barD change

stieg avatar May 30 '18 23:05 stieg

Regarding your first batch of cases, there's nothing wrong with your settings. The last case is not indented consistently with the rest. That's a bug. A PR to fix it is welcome.

Regarding your second batch of cases, I cannot reproduce the problem you report. If I run indent-region on that entire code, it indents correctly here. Even the last case, which is indented incorrectly without a return type in your first batch of cases, is indented correctly with the return type in the second batch. Make sure you are using the very latest version of typescript-mode was released. If you use Emacs' package facility, you have to get it from Melpa to get the latest version (or get it from the Github repo here). The version on Melpa Stable is quite out of date.

ETA: I saw the comment you posted on another issue mentioning version 0.2. Yeah, that's quite out of date. A lot of bad indentation cases have been fixed since that version.

lddubeau avatar May 30 '18 23:05 lddubeau

The last case is not indented consistently with the rest. That's a bug. A PR to fix it is welcome.

@lddubeau Thank you for confirming. I have not yet done any development on emacs to date but mayhaps now is the time to learn. Any pointers or good places to start would be most welcome.

Regarding your second batch of cases, I cannot reproduce the problem you report. ... you have to get it from Melpa to get the latest version (or get it from the Github repo here). The version on Melpa Stable is quite out of date.

Yes this was my issue. I was on Melpa stable. Moving to the more bleeding edge solved that issue. Thanks.

stieg avatar May 30 '18 23:05 stieg

In that regard, maybe we should consider publishing more builds on MELPA stable too. I'll make a v0.3 for now at least.

josteink avatar May 31 '18 07:05 josteink

In Emacs' help, there's "Emacs Lisp Intro" and "Elisp" which are useful.

The way I fix indentation issues is that I turn on EDebug (documented in the "Elisp" manual I mention above) on typescript--proper-indentation, and then run the indentation on the problematic case and step through the Lisp code to see where the logic goes astray.

The indentation code uses syntax-ppss everywhere, so you have to learn how that function works. (Documented in the "Elisp" documentation.)

Note that the way the indentation algorithm works, it starts from where the caret is located and looks backwards to get a clue as to what syntactic construct the caret is in. That's generally how Emacs modes do indentation. It works extremely well on simple syntaxes. For complex syntaxes, however, it makes the code very very hairy. TypeScript qualifies as "complex". The alternative to the current approach would be to perform indentation using an AST but there are significant issues with doing that.

lddubeau avatar May 31 '18 11:05 lddubeau

Thanks for the pointers @lddubeau . I'll start there and see where I can get to.

In that regard, maybe we should consider publishing more builds on MELPA stable too

@josteink That would be good too :D. At a minimum if you expect people to not use MELPA stable with this package it would be good to explicitly mention that in the README.md. Currently it notes that both are valid choices.

And most importantly.. thank you all for you work on this.

stieg avatar May 31 '18 19:05 stieg

Old issue is old. Typescript is in Emacs core now. Closing! :)

josteink avatar Oct 11 '23 08:10 josteink