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

feat(max-attributes-per-line): singleline.allowFirstLine option #1465

Merged

Conversation

privatenumber
Copy link
Contributor

@privatenumber privatenumber commented Mar 25, 2021

What rule do you want to change?
max-attributes-per-line

Does this change cause the rule to produce more or fewer warnings?
More

How will the change be implemented? (New option, new default behavior, etc.)?
Lower the minimum value on singleline/singleline.max to 0 on the schema.

Update:
Add a singleline.allowFirstLine option that defaults to true, but prevents single attributes from being on the same line if set to false.

Please provide some example code that this change will affect:
When setting singleline.max = 0, the following should be marked incorrect:

Update:
When setting singleline.allowFirstLine = false, the following should be marked incorrect:

<template>
	<component name="John Doe" />
</template>

What does the rule currently do for this code?
Currently, can't set singleline to 0 because of the schema validation.

What will the rule do after it's changed?
Correct it to:

<template>
	<component
		name="John Doe"
	/>
</template>

Additional context

  • This was working in prior versions of ESLint that did not enforce the JSON schema on options. But since upgrading ESLint, this appeared to be a regression. Since it's not a regression in this plugin, I labeled this as a feature request.

  • This is style used by a large codebase I'm contributing to.

@ota-meshi
Copy link
Member

Thank you for this PR!
I think it easier for users to add a singleline.allowFirstLine option similar to multiline.allowFirstLine option.
What do you think?

@privatenumber
Copy link
Contributor Author

Yeah that definitely makes more sense when trying to grasp the options.

Will update to allowFirstLine. Thanks!

@privatenumber
Copy link
Contributor Author

Now that I'm working on updating the docs, I'm second-guessing whether it makes sense.

It's a little strange that a "singleline" rule enforces the multiline behavior:

['error', {singleline: { allowFirstLine: false }}]
<template>
	<component
		name="John Doe"
	/>
</template>

I still think it's an improvement over max: 0 though.

@privatenumber privatenumber changed the title feat(max-attributes-per-line): allow max attribues 0 for singleline feat(max-attributes-per-line): singleline.allowFirstLine option Mar 26, 2021
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.

Thank you for changing this PR! LGTM

@ota-meshi ota-meshi merged commit 8bdb2a9 into vuejs:master Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants