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

Proposal: new-line-between-groups option for order #282

Closed
radekbenkel opened this issue Apr 27, 2016 · 10 comments
Closed

Proposal: new-line-between-groups option for order #282

radekbenkel opened this issue Apr 27, 2016 · 10 comments

Comments

@radekbenkel
Copy link
Contributor

This follow up on discussions we had here and #246.

Options for that would be either always or never.

Then for always and groups like: groups: [ [ "builtin", "external" ], [ "index", "sibling", "parent" ] ] }

So the following would be considered as a problem:

import url from 'url';
import express from 'express'; // <-- missing new line after this statement
import foo from './foo';

This would not be considered as a problem:

import url from 'url';
import express from 'express';

import foo from './foo';

new-line-between-groups would allow you to configure new lines inside groups like you want - should be pretty flexible.

I don't think there should be another rule for achieving that, option for order IMHO would be fine.

NOTE: Option name could of course be changed to something different, separate-groups(-with-newline) maybe.

@benmosher
Copy link
Member

I will mostly defer to @jfmengels and company on this one, but a few gut reactions:

I think if you don't set the option at all, it shouldn't enforce any spacing.

Also, if you do set it, to either setting, would it enforce that there are no newlines within a group?

i.e.

import url from 'url';
// assuming this were a blank line, would it be reported? "extra newline inside group" or some such
import express from 'express';

Also I think it would be optimal if the option were one or two words, something like newlines. Clarity over brevity, but brevity would be ideal.

@jfmengels
Copy link
Collaborator

I think if you don't set the option at all, it shouldn't enforce any spacing.

👍

if you do set it, to either setting, would it enforce that there are no newlines within a group?

I think so yes.

Maybe we could specify the number of blank lines between groups?
If I want no blank lines, I can set 0, if I want an empty line, I can set 1, and if I like bigger spacing, I can set 2. If I don't specify it or set it to false (?), nothing will be enforced.

if the option were one or two words, something like newlines

I think group-groups contains the wanted functionality, but it's a bit... repetitive ^^'

@radekbenkel
Copy link
Contributor Author

I think if you don't set the option at all, it shouldn't enforce any spacing.

Yep, then it's do whatever you want mode :)

if you do set it, to either setting, would it enforce that there are no newlines within a group?

Yes, this is what I had in mind.

Maybe we could specify the number of blank lines between groups?

Personally I don't think it's necessary, especially at the beginning. We might add it later :)

@jfmengels
Copy link
Collaborator

Maybe we could specify the number of blank lines between groups?

Personally I don't think it's necessary, especially at the beginning. We might add it later :)

One of the things it brings is make the option setting clearer IMO (well, depending on the name of the option...).

@benmosher
Copy link
Member

I like starting with true for 1 line, false for 0, and then later if it's int-valued that could be the number of lines, if someone wants that.

New name idea: newlines-between.

@jfmengels
Copy link
Collaborator

I think I like newlines-between. Just one thing, If you only allow at most 1 line and it's a boolean type field, then it should be newline-between, which doesn't upgrade well to an eventual int-type later. Man, naming is hard :p

@radekbenkel
Copy link
Contributor Author

@jfmengels Depends how to look at that - you can interpret it like: if you will have more than two groups then you will have newlineS between groups - even if it will be one line between group, total amount of empty lines in imports section will be bigger than 1, thus newlinesshould be fine.

I'm ok with plural form :)

@radekbenkel
Copy link
Contributor Author

So, can I assume it's something which would be ok with you guys and I can try to prepare a PR?

@benmosher
Copy link
Member

I'd wait for an explicit 👍 from @jfmengels but I'm good with where we ended up, discussion-wise.

@jfmengels
Copy link
Collaborator

Same here 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants