Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

two EmptyLines after import statements #975

Closed
stepancar opened this issue Feb 19, 2016 · 15 comments
Closed

two EmptyLines after import statements #975

stepancar opened this issue Feb 19, 2016 · 15 comments

Comments

@stepancar
Copy link

@stepancar stepancar commented Feb 19, 2016

hello! Thank you for this project.
How to add rule 'two newlines after import statements'?

import * as React from 'react';
import * as moment from 'moment';


export class TwoEmptyLinesBetweenCodeAndImportStatements {
}

Thank you!

@stepancar stepancar changed the title two newlines after import statements two EmptyLines after import statements Feb 19, 2016
@Pajn

This comment has been minimized.

Copy link
Contributor

@Pajn Pajn commented Feb 19, 2016

I had a custom rule (pre 3.0.0, haven't updated it) for blank lines above methods, may I propose a generic rule? Maybe something like:

"blank-lines": [true, {
  "after-imports": 2,
  "above-conditionals": 1,
  "above-methods": 1
}

where the numbers can allow different teams to configure their preferences.

@stepancar

This comment has been minimized.

Copy link
Author

@stepancar stepancar commented Feb 20, 2016

@Pajn good idea!

@rnfomin

This comment has been minimized.

Copy link

@rnfomin rnfomin commented Feb 20, 2016

@stepancar, @Pajn - This will be really nice rule. Thank you.

@eXod84

This comment has been minimized.

Copy link

@eXod84 eXod84 commented Feb 20, 2016

It will be great to have that rule. Thanks!

@myserge

This comment has been minimized.

Copy link

@myserge myserge commented Feb 20, 2016

Nice rule. Thank you.

@stepancar

This comment has been minimized.

Copy link
Author

@stepancar stepancar commented Feb 22, 2016

@Pajn could you share your custom rule, please?

@Pajn

This comment has been minimized.

Copy link
Contributor

@Pajn Pajn commented Feb 23, 2016

It's written by modifying a compiled rule, so it isn't super pretty.
https://gist.github.com/Pajn/ebd46aa0b38cf79ab29b

@atrauzzi

This comment has been minimized.

Copy link

@atrauzzi atrauzzi commented May 18, 2017

I definitely like this style and would love to see something offered in tslint's core for newlines after:

  • Imports (2)
  • Blocks (1)

Also would love this same mechanism to also enforce zero newlines when closing blocks.

@lucasklaassen

This comment has been minimized.

Copy link

@lucasklaassen lucasklaassen commented Jul 17, 2018

@Pajn any further movement on this?

@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Nov 3, 2018

IMO this doesn't seem like a good fit with TSLint core. We already have no-consecutive-blank-lines and have been slowly moving away from whitespace/formatting rules in favor of recommending Prettier. This also seems like unnecessary whitespace in the file that just makes it harder to read more of the file at once.

@stepancar / co., can you provide a justification for why this rule should exist?

@atrauzzi

This comment has been minimized.

Copy link

@atrauzzi atrauzzi commented Nov 4, 2018

Whoa, @JoshuaKGoldberg the idea here isn't to get your take on what is good or not. It also certainly isn't to force everyone into using prettier (my take is that it creates very ugly, overly-compact code).


Subjective

I'm not sure optimizing for "reading more of the file at once" is such a great idea either. I use two consecutive blank lines after import blocks to improve readability to allow blocks representing the static structure of my application to stand independent of regular statements. At a glance, this gives me a very easy way to move through multiple files, scanning their dependencies.


None of this means I want to impose the practice on others. At the same time though, I don't think it's unreasonable to ask that a linter support configurations specific to multiple organizations.

Think I'm gonna reach for the adage that what's good for the goose isn't good for the gander here.

@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Nov 4, 2018

@atrauzzi I'm sorry if my post came off too strong - removed the Resolution: Declined label from alongside Status: In Discussion.

For some more context, TSLint core's been moving away from formatting rules for some time now. Examples: #3995 / #3568 / #2814 / #3330 / #1306. In particular, per the discussion in #3330: every feature comes with a maintenance cost. The question here is whether the added maintenance for this formatting rule would be worth the benefit to users. How can we tell what's good for users? Normally TSLint goes by something like a casual mixture of collective gut intuition, well-explained use cases, and 👍 reactions to issues.

Suggestion: how about putting this in either tslint-consistent-codestyle or a separate repository + npm package? Having your own npm package in particular would let you iterate on it much more rapidly, instead of waiting for TSLint's release process.

@atrauzzi

This comment has been minimized.

Copy link

@atrauzzi atrauzzi commented Nov 4, 2018

I get that there's a lot of hype for prettier, but I think there's warranted skepticism towards the tool and its "opinions". I've looked them over, but I apologize, these linked issues don't appear to serve as explanation. At best, they seem like rushed reasoning. Prettier even says it in its project description: It's opinionated, so who is this project serving to point people in that direction?
If I know anything about our peers, you can usually count on most people to just give up and walk away than go through the toil of having to prove down their use case. For better and for worse. But either way, it's important when that happens to not think the argument for prettier was validated.

Anyway -- the truth is, any discussion about prettier itself is beside the point: tslint punting on formatting is like a restaurant saying it's not going to serve drinks anymore.
What is a linter if you take these formatting concepts away? I have no idea how to make custom rules and don't think tslint offers value if its users are required to delve into internals. So, that's something to consider when thinking about "what's good for users".

I - and many others I suspect - rely on tslint to be easy to use and a one-stop-shop.
Look at the number of issues linked to this one. Those are all probably worth 20 👍 apiece, plus any found within. 😉
Coming back to thinking about what's good for users, it would seem like it's time to put cognitive overhead and tooling bloat on the matrix and start weighing things out giving those principles some credence.

Note: None of this is to run afoul of seeming ungrateful. I do FOSS contributions where I think I'm most qualified and effective. The best I can offer here is critical discussion, so please -- don't take any of this wrong. I just think the minimalism here is starting to cut into the bone.

Thanks for hearing me out. 😄

@JoshuaKGoldberg

This comment has been minimized.

Copy link
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Aug 7, 2019

💀 It's time! 💀

TSLint is being deprecated and no longer accepting pull requests for major new changes or features. See #4534. 😱

If you'd like to see this change implemented, you have two choices:

  • Recommended: Check if this is available in ESLint + typescript-eslint
  • Not Recommended: Fork TSLint locally 🤷‍♂️

👋 It was a pleasure open sourcing with you!

If you believe this message was posted here in error, please comment so we can re-open the issue!

@pvinis

This comment has been minimized.

Copy link

@pvinis pvinis commented Aug 8, 2019

Time to make a new issue for eslint then :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.