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

jsx-max-props-per-line: autofix #949

Merged
merged 1 commit into from
May 19, 2017
Merged

jsx-max-props-per-line: autofix #949

merged 1 commit into from
May 19, 2017

Conversation

snowypowers
Copy link
Contributor

Hi this is my attempt at implementing #929. This aims to implement the fix such that a line with more than max props can be broken into more than 2 lines:

// Test case
<App a b c d e />

// [1]
<App a
b
c
d
e />

// [2]
<App a b
c d
e />

Would like an opinion on this if it is overkill. An alternate way of fixing would be under the simple fix commit (a simple addition of \n at a only one point).

Also, aware of #882 and hope to see some progress!

Copy link
Collaborator

@lencioni lencioni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a rebase, we could probably land this in the next release. Thanks for your contribution!

@snowypowers
Copy link
Contributor Author

Rebased. Looks like the new options doesn't really affect how the autofix will work so should be fine.
Added fix outputs to all the new invalid tests cases.

A second opinion is always welcomed :)

@kylpo
Copy link

kylpo commented Mar 20, 2017

I'd love for this to land soon! It will enable me to start using prettier (with prettier-eslint) and prevent it from messing with my max-props-per-line preference.

@snowypowers
Copy link
Contributor Author

Rebased as a single atomic commit.
@lencioni @yannickcr

@timhwang21
Copy link

Also would be happy for this to be released!

@kylpo
Copy link

kylpo commented Apr 18, 2017

I am just using @snowypowers's repo/branch until this lands. Working perfectly for my prettier-eslint setup, thanks @snowypowers!

@snowypowers
Copy link
Contributor Author

rebased again due to merge conflicts (mostly minor due to string templating changes)

@ljharb ljharb merged commit f0dcaca into jsx-eslint:master May 19, 2017
@revisionfour
Copy link

Thank you @snowypowers

You should find a $50 contribution in your paypal :)

@snowypowers
Copy link
Contributor Author

hi @revisionfour got it! Thanks, this PR has been floating for quite long, good to see people using it and finally, getting merged! ^.^

@revisionfour
Copy link

Yeah, agreed. Getting it merged in is huge because thats what really matters to the community as a whole. Sometime you have to bang on the repo maintainers door until they take a look and finally merge it in.

I'm hoping that the community continues to add fix commands to rules that can have them applied. I see eslint as an analogy to grammar/spelling correction in word processors. Most people can't remember when there was no spell or grammar checker when they went to write a paper. Right now in my opinion, writing code is still in its infancy because we should have a best standards correction always on when we code. This is vital to getting government and large teams to write consistent and readable code, while reducing bugs and improving efficiency and meeting deadlines. Many programmers believe that using eslint is for amateurs and these programmers are highly misguided by their egos. I would ask these programmers to write me a 10 page paper about the future of coding from start to finish without spelling or grammar checking on and with no revisions. How many programmers could do this without 1 spelling or grammer error. None. eslint is vital to getting programming into the hands of the masses.

@ljharb
Copy link
Member

ljharb commented May 19, 2017

Not every eslint rule is programmatically fixable; many require human intervention and always will - but that's part of why it's so important to make all the ones that are fixable, fixable.

Thanks for the contribution!

@pasupuletics
Copy link

@revisionfour, When will be next release going to happen?

CC: @snowypowers

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

Successfully merging this pull request may close these issues.

7 participants