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

Create vue/multiline-ternary extension rule #1996

Merged
merged 17 commits into from Oct 10, 2022

Conversation

dev1437
Copy link
Contributor

@dev1437 dev1437 commented Sep 28, 2022

Wrap multiline ternary rule for vue.

Not sure about the indenting, maybe it is not an issue and would be handled by other rules. Looks strange.

@FloEdelmann
Copy link
Member

FloEdelmann commented Sep 29, 2022

@dev1437 Could you please fix the lint issues? Also, there are some tests failing in ESLint v6, @ota-meshi maybe you can help how to skip those tests there?

@dev1437
Copy link
Contributor Author

dev1437 commented Sep 29, 2022

Not sure why the test fail in eslint v6 but run fine locally, but lint is okay now

@ota-meshi
Copy link
Member

It seems that the multiline-ternary rule did not previously support autofix.

I think it is necessary to check the version of eslint and ignore the output check (set null).
I think the following test case will be helpful.
https://github.com/vuejs/eslint-plugin-vue/blob/master/tests/lib/rules/no-loss-of-precision.js#L20

lib/rules/multiline-ternary.js Outdated Show resolved Hide resolved
@FloEdelmann

This comment was marked as resolved.

@FloEdelmann FloEdelmann changed the title Create multiline ternary rule Create vue/multiline-ternary extension rule Sep 30, 2022
tests/lib/rules/multiline-ternary.js Outdated Show resolved Hide resolved
lib/rules/multiline-ternary.js Show resolved Hide resolved
Copy link
Member

@ota-meshi ota-meshi left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@ota-meshi
Copy link
Member

#1996 (comment)

tests/lib/rules/multiline-ternary.js Outdated Show resolved Hide resolved
docs/rules/multiline-ternary.md Outdated Show resolved Hide resolved
Copy link
Member

@FloEdelmann FloEdelmann left a comment

Choose a reason for hiding this comment

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

Thanks for implementing all the feedback!

@ota-meshi ota-meshi merged commit 4d79ceb into vuejs:master Oct 10, 2022
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

Successfully merging this pull request may close these issues.

None yet

3 participants