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

Compact groups #454

Open
QuentinRoy opened this issue Aug 23, 2017 · 3 comments
Open

Compact groups #454

QuentinRoy opened this issue Aug 23, 2017 · 3 comments

Comments

@QuentinRoy
Copy link

I like the option to automatically group my imports by categories, but I am not a big fan of the whitespaces between groups. I haven't find a way to strip them and keep my imports grouped but compact. I wish we could have an option for this.

@trotzig
Copy link
Collaborator

trotzig commented Aug 23, 2017

I agree that the current behavior is a little inflexible. Can you explain what you want the imports to look like? We've discussed making the groups configurable in the past and I think it's mostly waiting for someone to write a PR. I haven't seen whitespace removal come up before, but I'm sure you have good reasons.

@QuentinRoy
Copy link
Author

QuentinRoy commented Aug 24, 2017

Simple, instead of this:

import React, { Component } from 'react';
import propTypes from 'prop-types';

import { ContextSwitch, Context, contextPropType } from 'reacolo';

import ControlApp from './control/ControlApp';
import LoadingView from './LoadingView';
import XpApp from './display/XPApp';

class BaseApp extends Component {
  constructor(props) {
    super(props);
    /* stuff */
  }

  render() {
    return <div>stuff</div>;
  }
 }

I prefer this

import React, { Component } from 'react';
import propTypes from 'prop-types';
import { ContextSwitch, Context, contextPropType } from 'reacolo';
import ControlApp from './control/ControlApp';
import LoadingView from './LoadingView';
import XpApp from './display/XPApp';

class BaseApp extends Component {
  constructor(props) {
    super(props);
    /* stuff */
  }

  render() {
    return <div>stuff</div>;
  }
 }

While I like sorting the imports per categories, I don't feel it benefits much to add new lines between this categories. However I find it makes it harder to find the separation between the imports and the rest of the code (which for me is important).
Someone might also want to define what to separate or not, e.g. I could do with local import / global import, but the separation between named import and default import does not make much sense to me (in particular as they can be mixed, c.f. the first line).
My suggestion would actually to treat "grouping" and "alphabetical sort" as two different ways of sorting the imports: I find having two properties for this a bit confusing, what if I put "grouping" and "sorting" both to true ? Then deal with new line insertions as a separate option.

@trotzig
Copy link
Collaborator

trotzig commented Aug 24, 2017

Thanks for providing that context. I agree that it's a little odd to have both those configuration options. I think the reason is that they were added one by one gradually, and we never took a holistic look at things.

Providing a configuration option to disable the whitespace shouldn't be too hard I think. If that's something you'd find useful I'll be happy to review a PR.

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

No branches or pull requests

2 participants