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 --globby-options flag #6109

Closed
silversonicaxel opened this issue May 24, 2022 · 22 comments · Fixed by #6437
Closed

Add --globby-options flag #6109

silversonicaxel opened this issue May 24, 2022 · 22 comments · Fixed by #6437
Labels
status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules

Comments

@silversonicaxel
Copy link

What steps are needed to reproduce the bug?

Create a folder beginning with a dot, and set a css or scss file and run a stylelint --fix command. It won't work.
I'm experiencing this problem with .storybook folder.

What Stylelint configuration is needed to reproduce the bug?

any configuration would be enough

How did you run Stylelint?

stylelint --fix '**/*.css

Which version of Stylelint are you using?

13.13.1

What did you expect to happen?

The lint fixes also files present in hidden folders

What actually happened?

Hidden folders files are ignored

Does the bug relate to non-standard syntax?

No

Proposal to fix the bug

Read also hidden folders

@ybiquitous
Copy link
Member

ybiquitous commented May 24, 2022

@silversonicaxel Thanks for reporting the issue and using the template.

Stylelint uses internally globby and fast-glob to resolve specified globs, and fast-glob ignores dot files by default (see the dot option).

I think you can specify dot files like this:

stylelint '**/*.css' '.*/**/*.css'

See also https://stylelint.io/user-guide/usage/node-api/#globbyoptions

@silversonicaxel
Copy link
Author

Hello @ybiquitous , I've a question, will it be solved soon the issue? or not really?
I was able to fix it, but the command now looks terrible, since I've some hidden folders in a subfolder, so I had to explicitly mention it like this

stylelint --fix '**/*.scss' '.*/**/*.scss' 'deep/in/the/code/subfolder/.*/**/*.scss'

it works, it is simply not so neat

@ybiquitous
Copy link
Member

@silversonicaxel Thanks for sharing the pain. Indeed, it's a bit ugly.

It may be good to add a CLI option to traverse dotfiles, but I'm unsure.
(for example, it doesn't make sense to traverse .git/** files)

I'll let the issue open for further discussion.

@ybiquitous ybiquitous added the status: needs discussion triage needs further discussion label May 24, 2022
@alexander-akait
Copy link
Member

alexander-akait commented May 24, 2022

@ybiquitous We should not lint . files, they are hidden files and it is good by default avoid to lint them (that is why glob by default doesn't collect . files too), so using '.*/**/*.scss' is a valid solution. Even more linux (and more) don't show . files in your file explorer, I strongly reccomend don't use . directories (if you really don't want to hide something), it can be a problem for developers on other platforms/os

@silversonicaxel
Copy link
Author

My folder is .storybook, obviously it is considered hidden and I strongly believe I can't rename it. Inside that folder I've also some custom css, that I desire to lint.

So I also understand @alexander-akait point of view. In my opinion this should be also a config parameter, so users can decide their preferences.

What do you think?

@silversonicaxel
Copy link
Author

For the .git we could eventually use stylelintignore file @ybiquitous

@alexander-akait
Copy link
Member

I am fine with an additional option

@silversonicaxel
Copy link
Author

silversonicaxel commented May 24, 2022

Who's going to be in charge of this implementation? How does it work?
I could potentially help, but, if someone of stylelint team is willing to give precise insights, I could also help in a possible PR.

@ybiquitous
Copy link
Member

ybiquitous commented May 25, 2022

@alexander-akait @silversonicaxel
Thanks for the feedback. I also agree with ignoring dotfiles by default (the same as always).

So, it seems we need to consider a new CLI option to pass { dot: true } to globby to address the following use-cases:

$ tree .foo
.foo/
├── .bar/
│   └── a.css
└── a.css

1 directory, 2 files

$ npx stylelint '.foo/**/*.css'

.foo/a.css
 1:12  ✖  Unexpected unit  length-zero-no-unit

In the example above, we should want to check not only .foo/a.css but also ./foo/.bar/a.css.

Decisions required for the new option if needed:

  • name (e.g. --dot)
  • argument (e.g. --dot=true, --dot=false, or disallows an argument?)
  • default value (e.g. [default: false])
stylelint '.foo/**/*.css' --dot
# should match `.foo/a.css` and `.foo/.bar/a.css`

stylelint '**/*.css' --dot --ignore-pattern '.git/**'
# should be useful with `.stylelintignore` or `--ignore-pattern`

Any thoughts?

@silversonicaxel
Copy link
Author

I would like to have this command in my script

stylelint '**/*.css' --dot
# should match `.foo/a.css` and `.foo/.bar/a.css`

and regarding this

stylelint '**/*.css' --dot --ignore-pattern '.git/**'

could also live in .stylelintignore as

node_modules
dist
.git
.vscode

but then it is up to the consumers who are into hidden folders to set them in the ignore list

to me looks good like this!

@jeddy3
Copy link
Member

jeddy3 commented Jun 6, 2022

We already expose the globby options object to the Node.js API. Can you make it available to the CLI too? For example:

stylelint '**/*.css' --globby-options "dot: false"

It'll save us adding a new option for this edge case.

@ybiquitous
Copy link
Member

Considering consistency with the Node.js API, --globby-options is also a good option. 👍🏼

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules and removed status: needs discussion triage needs further discussion labels Jun 6, 2022
@jeddy3 jeddy3 changed the title It seems stylelint does not enter hidden folders (beginning with .) Add --globby-options flag Jun 6, 2022
@jeddy3
Copy link
Member

jeddy3 commented Jun 6, 2022

Let's an --globby-options flag.

I could potentially help... Who's going to be in charge of this implementation? How does it work?

It's great you want to help as a new feature is generally implemented by the person who requested it or by someone who also needs it. The team working on Stylelint have time to review pull requests, but not time to implement features like this unless they need it themselves.

@silversonicaxel I've labelled the issue as ready to implement. Please consider contributing if you have time.

@jeddy3 jeddy3 changed the title Add --globby-options flag Add --globby-options flag Sep 28, 2022
@sidverma32
Copy link
Contributor

@jeddy3 @ybiquitous is this done or shall i pick this up?

@ybiquitous
Copy link
Member

@sidverma32 Thank you! Nobody currently works on this issue, so please feel free to create a pull request.

@sidverma32
Copy link
Contributor

Can you guide me which file to refer to add this option support? @ybiquitous

Also i can see this dot support present in code repo, would like to know if we have the support for dot but not utilising it properly or is this dot referring to some other functionality?

Screenshot 2022-10-29 at 12 25 18 PM

@silversonicaxel
Copy link
Author

thanks, I also had issues to implement myself this interesting feature. kudos to volounteers!

@ybiquitous
Copy link
Member

@sidverma32 To support the --globby-options CLI flag, we need to change lib/cli.js and pass the new flag value to standalone():

return standalone(options)

lib/standalone.js has allowed the globbyOptions option:

globbyOptions,

@sidverma32
Copy link
Contributor

@sidverma32 To support the --globby-options CLI flag, we need to change lib/cli.js and pass the new flag value to standalone():

By new flag value you mean something like this :
return standalone(options, newFlag)

or something like this:

if(cli.flags.globbyOptions){
  optionBase.globbyOptions = cli.flags.globbyOptions;
}

@ybiquitous
Copy link
Member

I mean:

if (cli.flags.globbyOptions) {
  optionBase.globbyOptions = cli.flags.globbyOptions;
}

@sidverma32
Copy link
Contributor

I mean:

if (cli.flags.globbyOptions) {
  optionBase.globbyOptions = cli.flags.globbyOptions;
}

Thanks for clarification! Please have a look over the raised PR and let me know if i missed any detail.

@silversonicaxel
Copy link
Author

I tried the upgraded version of stylelint

#6437 (comment)

but I still have the same issue and I wanted to share it again here.

infotexture added a commit to infotexture/dita-bootstrap.extension that referenced this issue Jan 8, 2024
Stylelint ignores hidden folders by default, so we need to pass --globby-options '{"dot":true}' to ensure that CSS files in `.github` workflow folders are checked.

For background, see:
- stylelint/stylelint#6109
- stylelint/stylelint#6437 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules
Development

Successfully merging a pull request may close this issue.

5 participants