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 --rule flag, --rules flag, --merge-rules flag and mergeRules option #7252

Closed
wants to merge 3 commits into from

Conversation

Mouvedia
Copy link
Member

@Mouvedia Mouvedia commented Oct 19, 2023

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

#4331

  • standalone
  • cli
  • documentation
  • tests

Is there anything in the PR that needs further explanation?

levn has limited uses; I could refactor and use JSON.parse instead.
e.g. having a schema for an object requires explicit property names

missing features

  • value-less rule selection

Passing a rule without its value seemed confusing.
What should --rule color-no-invalid-hex do exactly?
Probably fetch the corresponding rule option in the config and, if found, discard all other rules.
I chose not to add this feature, for now; it needs to be discussed in a separate issue.

  • --rules should accept a file path (JSON)

@changeset-bot
Copy link

changeset-bot bot commented Oct 19, 2023

🦋 Changeset detected

Latest commit: 61e9858

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
stylelint Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

lib/cli.mjs Fixed Show fixed Hide fixed
lib/cli.mjs Fixed Show fixed Hide fixed
lib/cli.mjs Fixed Show fixed Hide fixed
@Mouvedia Mouvedia marked this pull request as ready for review October 19, 2023 18:03
@Mouvedia Mouvedia changed the title Add --rule and --merge-rules Add --rule flag, --merge-rules flag and mergeRules option Oct 19, 2023
@ybiquitous
Copy link
Member

@Mouvedia Can you please wait until the v16 branch is merged into main? Otherwise, we would have a pain to have to resolve so many conflicts.

@Mouvedia

This comment was marked as outdated.

@ybiquitous

This comment was marked as outdated.

@Mouvedia

This comment was marked as outdated.

@Mouvedia Mouvedia requested review from romainmenke and mattxwang and removed request for ybiquitous October 20, 2023 00:19
@mattxwang

This comment was marked as outdated.

@Mouvedia

This comment was marked as outdated.

@Mouvedia Mouvedia marked this pull request as draft December 8, 2023 17:30
@ybiquitous
Copy link
Member

@Mouvedia You can restart this. But I'm concerned about where the --merge-rules idea comes from.

@Mouvedia Mouvedia changed the title Add --rule flag, --merge-rules flag and mergeRules option Add --rule flag, --rules flag, --merge-rules flag and mergeRules option Dec 19, 2023
@Mouvedia
Copy link
Member Author

Closed in favour of #7418.
@ybiquitous I am planning to finish it this week.

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.

None yet

3 participants