dprint / dprint-plugin-typescript

TypeScript and JavaScript code formatting plugin for dprint.
https://dprint.dev/plugins/typescript
MIT License
248 stars 50 forks source link

Blank lines option #7

Open spinlud opened 4 years ago

spinlud commented 4 years ago

Is possible to have an option to add blank line rule(s) in the formatter? Eg transform this

class Temp {
    @Dec1
    prop1! int;
    @Dec2
    prop2! string;
}

to this:

class Temp {
    @Dec1
    prop1! int;

    @Dec2
    prop2! string;
}

or this:

class Temp {

    @Dec1
    prop1! int;

    @Dec2
    prop2! string;

}
lazarljubenovic commented 4 years ago

Relevant:

https://eslint.org/docs/rules/lines-between-class-members https://eslint.org/docs/rules/padded-blocks

Might be of some help, since they both have fixes.

dsherret commented 4 years ago

Thanks @lazarljubenovic! I appreciate the "prior art" in order to get some ideas 🙂

Right now only member spacing is implemented for enum members. The options are "maintain", "newline", and "blankline" (I believe). I could add this to other nodes and make a general setting.

Also, I can add a "padding blocks" option. I'm just going to get dprint/dprint#54 merged before doing any of this though.

kkoudev commented 1 year ago

@dsherret Any update on this?

B4nan commented 1 year ago

Is this something you still consider? Is there anything we can help with to move it forward?

I think it would be a great way to attract many new users (like myself) who can't stand how prettier works when it comes to blank lines, but still want to use some formatter. The problem is especially visible when you use decorators, but it's not just about that. Here is a relevant prettier issue: https://github.com/prettier/prettier/issues/4870, it's funny how they suggest using eslint instead if this bothers you, while the TS eslint version has completely broken indent rule, and they point in the very opposite direction - don't use our broken indent rule, use a formatter instead.

lazarljubenovic commented 8 months ago

I love coming to an issue and being greeting with my own comment 😄

Experimenting with dprint again; would love to see this option. Here's a list of my mental rules for formatting -- it might help decide on which options to add.

  1. Obligatory exactly two blank lines after all imports. Allow at most one blank line between groups of imports.
import a from 'a'
import b from 'b'

import x from './x'
import y from './y'

console.log()
  1. Inside blocks, allow at most one blank line between things.
function () {
  foo()
  bar()
}
  1. If the block has at least one empty line, then padding empty lines are necessary.
function () {

  // first this
  foo()
  bar()

  // then this
  foo()
  bar()

}

That way, a simple class could be written as

class Foo {
  bar: string
  baz: number
}

but a more complex one would, due to blank lines between properties, force the padded lines.

class Foo {

  /** docs */
  bar: string

  /** docs */
  baz: string

}

Bonus points: anything multi-lined in a class forces blank lines around it (which in turn forces the padding.

I love the topic of formatting code. I probably sound crazy to some with the rules I follow, and yet I had to ask myself really hard to define how I format code manually (and subconsciously) to derive all the rules up there 🤔

Bessonov commented 6 months ago

I am currently experimenting with dprint, and it appears to be superior to many options I have tried so far, such as eslint, prettier, oxc, and biome. The "AST configuration" feature is particularly impressive. dprint is almost perfect and lacks only a few options, and this is one of them.

I largely agree with the proposal made by @lazarljubenovic in https://github.com/dprint/dprint-plugin-typescript/issues/7#issuecomment-1778074041. However, I'm not entirely convinced by the two-blank lines part.

Additionally, I would like to suggest an option to preserve single empty lines and remove only redundant ones. With the AST configuration, this enhancement would make dprint even more powerful.

Bessonov commented 6 months ago

@lazarljubenovic for the two-blank lines part you are, probably, interested in https://github.com/dprint/dprint-plugin-typescript/issues/394

Bessonov commented 5 months ago

@dsherret after thinking about the two blank lines proposed earlier, I have come to the conclusion that the best option would be something like 'maintain' | number, where the number represents the count of line breaks. 0 would mean no line break, 1 would represent one line break, and so on. Together with the AST configuration, this would be quite powerful.

bschaepper commented 2 months ago

This project looks amazing, I'd use it over prettier/biome in a heartbeat once this feature is done.

todor-a commented 1 month ago

Is there a vision on how we want this to look like? I would love to give implementing it a go.

@dsherret

bschaepper commented 1 week ago

While I prefer two lines separating imports from the following code, it doesn't matter all that much to me.

Two cases are a deal breaker though. Class padding, and empty lines in tests.

Here's what I'd like to see:

// foo.ts
class Foo {

    public constructor() {
        // ...
    } 

}

// foo.spec.ts
import { describe, it } from "my-testing-framework"

describe('Foo Test', () => {

    it('should do the thing', () => {
        arrange()

        act()

        assert()
    })

    it('should do the thing without arranging', () => {

        act()

        assert()
    })

})

Rather than that:

// foo.ts
class Foo {
  public constructor() {
    // ...
  }
}

// foo.spec.ts
import { describe, it } from "my-testing-framework"

describe("Foo Test", () => {
  it("should do the thing", () => {
    arrange()

    act()

    assert()
  })

  it("should do the thing without arranging", () => {
    act()

    assert()
  })
})

The class padding doesn't seem very useful in this small example, but is so much better as soon as there are generics, constructor arguments or base classes involved.

The tests are too dense even in this small example. It suffers from visual closeness where it should be (describe, it, arrange), but has visual stops inside test cases instead. Also, I like to structure tests in arrange/act/assert, and split them by a newline. If there is no setup code, I leave a blank line to make it clear.

I did implement something (even though my Rust skills are zero, and I don't understand how dprint works at all. It's pretty gruesome, but maybe helps the discussion). I think it might be good enough for my taste, to always have padding. Sure, simple classes don't really need it, but rather this than the opposite.

I fell like a perfect solution would allow me to configure a whitelist of keywords/function names that allow for leading blank lines:

  "allowLeadingBlanks": "/class|describe|it/",

(This would not solve blank lines at the end of a class body, but I could live with that)