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

Add invalid rule primary option message for false #6250

Merged
merged 2 commits into from Aug 11, 2022

Conversation

Mouvedia
Copy link
Contributor

@Mouvedia Mouvedia commented Aug 8, 2022

Which issue, if any, is this issue related to?

Closes #5091

see also #2027 #5221 #3516 #1953

Is there anything in the PR that needs further explanation?

check the message of the first commit
101f274#diff-ea40f7b7d9827960e2d8e1590b37c7490648acb555d8fe8c22d0c2328449c95fR64

@Mouvedia
Copy link
Contributor Author

Mouvedia commented Aug 8, 2022

This line
for (const a of [actual].flat()) {
in validateOptions.js motivated the config change.
Don't squash the commits so that it can be reverted easily.

@Mouvedia
Copy link
Contributor Author

Mouvedia commented Aug 8, 2022

@jeddy3 this line says

Validate a rule's options.

so it shouldn't be called to validate options of things that are not rules, correct?
i.e. validateDisableSettings calls validateOptions internally

For now Ill add exceptions for reportDescriptionlessDisables, reportNeedlessDisables and reportInvalidScopeDisables which all accept false as a primary option.

@Mouvedia Mouvedia force-pushed the mouvedia-5091 branch 2 times, most recently from 2f17962 to 108a9a3 Compare August 8, 2022 13:25
@ybiquitous
Copy link
Member

@Mouvedia Thanks for creating the PR!

The 66cdca0 commit looks good to me. 👍🏼

Don't squash the commits so that it can be reverted easily.

We usually "squash and merge", so if you open a new PR including the commit above, I'll approve and merge it soon.

@Mouvedia Mouvedia changed the title Fix #5091 and tsconfig.json Fix #5091 ~and tsconfig.json~ Aug 9, 2022
@Mouvedia Mouvedia changed the title Fix #5091 ~and tsconfig.json~ Fix #5091 Aug 9, 2022
@Mouvedia Mouvedia mentioned this pull request Aug 9, 2022
@Mouvedia
Copy link
Contributor Author

Mouvedia commented Aug 9, 2022

@ybiquitous is

possible: [true, false],

a problem?

@ybiquitous
Copy link
Member

Ah, possible: [true, null] is less confusing. Can you fix it also, please?

@ybiquitous ybiquitous changed the title Fix #5091 Fix invalid option message for false Aug 9, 2022
@ybiquitous ybiquitous changed the title Fix invalid option message for false Fix invalid rule option message for false Aug 9, 2022
@Mouvedia
Copy link
Contributor Author

Mouvedia commented Aug 9, 2022

Ah, possible: [true, null] is less confusing.

Nop: Type 'boolean' is not assignable to type 'RuleOptionsPossible[]'.

Can you fix it also, please?

Yup.

@Mouvedia Mouvedia changed the title Fix invalid rule option message for false Add invalid rule primary option message for false Aug 10, 2022
Copy link
Member

@ybiquitous ybiquitous 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. LGTM! 👍🏼

@ybiquitous ybiquitous merged commit 096113e into stylelint:main Aug 11, 2022
@ybiquitous
Copy link
Member

Changelog entry added:

  • Added: invalid rule primary option message for false (#6250).

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

Successfully merging this pull request may close these issues.

Fix invalid option message for false for better DX
2 participants