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.

Sign up for GitHub

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

Jump to bottom

Add --ext CLI flag and extensions Node.js API option #6617

Closed
wants to merge 2 commits into from
Closed

Add --ext CLI flag and extensions Node.js API option #6617

wants to merge 2 commits into from

Conversation

Mouvedia
Copy link
Member

@Mouvedia Mouvedia commented Feb 6, 2023 •
edited

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

Closes #6689

  • --ext flag
    • list
    • '.css' default
    • documentation
    • test
  • extensions Node.js
    • array
    • documentation
    • extends
    • test
      • extensions
      • extends
      • prefix
      • .
  • fixturePath test util

Is there anything in the PR that needs further explanation?

I used ESLint as a reference.
see #6617 (comment)
I am not planning to cover RFC20 in this first iteration, let's keep it simple.

related upstream issue: sindresorhus/meow#164

@changeset-bot
Copy link

changeset-bot bot commented Feb 6, 2023 •
edited

🦋 Changeset detected

Latest commit: a77ac8d

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

Mouvedia
Mouvedia commented Feb 6, 2023
View reviewed changes
lib/cli.js Outdated Show resolved Hide resolved
@Mouvedia Mouvedia changed the title [DRAFT] placeholder title Add ext flag and extensions property Feb 6, 2023
@Mouvedia Mouvedia force-pushed the mouvedia-ext branch 3 times, most recently from 1345fa9 to 2e4984f Compare February 6, 2023 17:34
@Mouvedia Mouvedia marked this pull request as ready for review February 6, 2023 17:35
@Mouvedia Mouvedia force-pushed the mouvedia-ext branch 5 times, most recently from d6fb067 to 57c950b Compare February 7, 2023 10:09
@Mouvedia Mouvedia marked this pull request as draft February 7, 2023 11:25
@Mouvedia Mouvedia force-pushed the mouvedia-ext branch 5 times, most recently from 2495867 to 81a4e17 Compare February 7, 2023 16:14
@Mouvedia Mouvedia marked this pull request as ready for review February 7, 2023 16:14
@Mouvedia Mouvedia requested a review from jeddy3 February 7, 2023 16:14
@Mouvedia Mouvedia force-pushed the mouvedia-ext branch 2 times, most recently from 978828f to 0173c41 Compare February 7, 2023 16:24
@Mouvedia Mouvedia mentioned this pull request Feb 9, 2023
Closed
@Mouvedia Mouvedia linked an issue Feb 9, 2023 that may be closed by this pull request
Clean test output #6631
Closed
@Mouvedia Mouvedia removed a link to an issue Feb 9, 2023
Clean test output #6631
Closed
ybiquitous
ybiquitous reviewed Mar 1, 2023
View reviewed changes
lib/standalone.js Outdated Show resolved Hide resolved
@Mouvedia Mouvedia marked this pull request as draft March 1, 2023 17:57
@Mouvedia Mouvedia force-pushed the mouvedia-ext branch 6 times, most recently from 6677b12 to 20e50d2 Compare March 2, 2023 02:46
@Mouvedia Mouvedia marked this pull request as ready for review March 2, 2023 02:52
@Mouvedia Mouvedia marked this pull request as draft March 2, 2023 03:03
@Mouvedia Mouvedia force-pushed the mouvedia-ext branch 2 times, most recently from d72ec44 to 9046a08 Compare March 2, 2023 03:33
@Mouvedia Mouvedia marked this pull request as ready for review March 2, 2023 03:40
@silverwind
Copy link

silverwind commented Mar 2, 2023 •
edited

Can the leading dot be made optional, so that css,scss works the same as .css,.scss? This is matching behaviour of the same eslint option.

@Mouvedia
Copy link
Member Author

Mouvedia commented Mar 2, 2023 •
edited

Can the leading dot be made optional, so that css,scss works the same as .css,.scss? This is matching behaviour of the same eslint option.

I guess we could. If we do add it we would need a new test and documentation update(s).
I think it already works. It's only an easter egg right now.

  • new test
  • documentation update

@Mouvedia Mouvedia force-pushed the mouvedia-ext branch 2 times, most recently from bf85689 to b1292c7 Compare March 3, 2023 01:33
@Mouvedia Mouvedia requested a review from ybiquitous March 3, 2023 02:12
ybiquitous
ybiquitous reviewed Mar 3, 2023
View reviewed changes
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.

Thanks for the hard work. The behavior seems no problems.
I've suggested a few comments for a bit of improvement and maintainability. Please consider addressing them.

lib/cli.js Outdated Show resolved Hide resolved
lib/__tests__/standalone-ext.test.js Outdated Show resolved Hide resolved
lib/__tests__/standalone-ext.test.js Outdated Show resolved Hide resolved
lib/__tests__/standalone-ext.test.js Show resolved Hide resolved
@Mouvedia Mouvedia requested a review from ybiquitous March 4, 2023 02:46
@Mouvedia Mouvedia added the pr: blocked is blocked by another issue or pr label Mar 10, 2023
@sidverma32 sidverma32 mentioned this pull request Mar 20, 2023
Merged
@Mouvedia Mouvedia closed this Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hudochenkov hudochenkov hudochenkov left review comments

@ybiquitous ybiquitous Awaiting requested review from ybiquitous

Assignees
No one assigned
Labels
pr: blocked is blocked by another issue or pr
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add --ext flag and corresponding Node.js API option
5 participants
@Mouvedia @ybiquitous @romainmenke @silverwind @hudochenkov