Cannot run lint or test suite; always fails on lint:prettier

[rjgotten]

• Operating System: Windows 10
• Node Version: 10.x
• NPM Version: 9.6
• webpack Version: -
• mini-css-extract-plugin Version: -

Expected Behavior

When developing a pull request and using npm run lint or npm run test the linter or full test suite is run and reports correct results without false positives.

Actual Behavior

When the lint:prettier task is hit, it results in all scanned files being marked as modified - even though they are not.

How Do We Reproduce?

Check out the plugin's repository on a Windows system. Do not make any code changes. Only execute npm run lint or npm run test. Watch the lint:prettier task still generate a long list of differences and then exit with error code 1, blocking the remainder of the linting or test suite from running.

[alexander-akait]

npm run lint output file(s) what you need fix, so please run npm run lint -- --write to fix problem, or install prettier as plugin for your IDE

[rjgotten]

@evilebottnawi

Yeah; that doesn't do anything.

PS D:\github_git\mini-css-extract-plugin> npm run lint -- --write

> mini-css-extract-plugin@0.8.0 lint D:\github_git\mini-css-extract-plugin
> npm-run-all -l -p "lint:**" "--write"

ERROR: Invalid Option: --write

You are also missing the point: Every file in the repository which is scanned by Prettier is output as needing to be fixed, straight after checking out the repository and not having made a single change (!!) to anything...

So; either your Prettier configuration is wrong and its delivering a long list of false positives, or your actual codebase doesn't adher to its own linting standards.

[alexander-akait]

npm run lint:prettier -- --write

[alexander-akait]

You are also missing the point: Every file in the repository which is scanned by Prettier is output as needing to be fixed, straight after checking out the repository and not having made a single change (!!) to anything...

So; either your Prettier configuration is wrong and its delivering a long list of false positives, or your actual codebase doesn't adher to its own linting standards.

What? We run linter on CI, this setup using for all projects in webpack/webpack ecosystem, so you do something wrong

[alexander-akait]

lint !== fix

[alexander-akait]

Maybe because you use windows and you automatically change linux end of line on windows and maybe before working on repo you need minimum setup for git?

[rjgotten]

Maybe because you use windows and you automatically change linux end of line on windows and maybe before working on repo you need minimum setup for git?

Or maybe, instead of telling people to change their global git setup, you enforce line-endings in the .gitattributes file of your own repository?

[EDIT] Oh, wait. Look what I found:

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/0bacfac7ef4a06b4810fbc140875f7a038caa5bc/.gitattributes#L2

Maybe actually remove that or change it to lf, ey?

[alexander-akait]

@rjgotten maybe somebody can't be toxic and look in repos and find what editorconfig exists https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/.editorconfig and it is standard for repositories?

[alexander-akait]

Maybe actually remove that or change it to lf, ey?

No because tests cases can contains LF/CRLF/CR line ending for testing

[rjgotten]

You mean the testcases that are already all using LF?

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/0bacfac7ef4a06b4810fbc140875f7a038caa5bc/.gitattributes#L3

[alexander-akait]

source code also potentially can have lines (example literals) with other line ending, look in big repos, for example react https://github.com/facebook/react/blob/master/.gitattributes

[alexander-akait]

But editorconfig use lf https://github.com/facebook/react/blob/master/.editorconfig#L6

[alexander-akait]

There are a lot of guide for for beginners about organization repos, please read them before you start arguing, it makes no constructive thoughts. Also you can send a PR in free style and when fix problem based on CI report or you can leave as is and ask the contributors to fix it, because you can't do it yourself, there is nothing shameful

[ThiefMaster]

I have no idea what the issue is but I think eol=lf makes lots of sense for text/code files. If you really need crlf for some files, then override those specifically. But auto uses whatever line ending the OS uses, so that doesn't seem like a good choice unless someone uses an editor that doesn't work well with different line endings.

[alexander-akait]

@ThiefMaster react has invalid repo setup too, right?

[ThiefMaster]

https://github.com/facebook/react/commit/8abca77381fbd31ffc2ff2b4fa12020768b8fc07 is the commit from 5 years ago, and the commit message sounds more like they wanted =lf unless they prefer people getting CRLF line endings when cloning the repo on windows ;)

[alexander-akait]

@ThiefMaster because you should setup git configuration before working locally, this is described in all the manuals for working with git in some detail

[alexander-akait]

https://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line/

One final note that the man page for gitattributes mentions is that you can tell git to detect all text files and automatically normalize them (convert CRLF to LF):

• text=auto This is certainly better than requiring everyone to be on the same global setting for core.autocrlf, but it means that you really trust Git to do binary detection properly. In my opinion it is better to explicitly specify your text files that you want normalized. Don’t forget if you are going to use this setting that it should be the first line in your .gitattributes file so that subsequent lines can override that setting.

[rjgotten]

because you should setup git configuration before working locally

I have core.autocrlf=false set in global git configuration exactly to keep all checked out repositories' original line-endings.

This repository specifically sets text=auto in .gitattributes and overrules that behavior, making Git normalize line-endings to the OS standard. Which is CRLF on Windows.

[alexander-akait]

@rjgotten based on you opinion, react use invalid configuration too, right?

[rjgotten]

@rjgotten based on you opinion, react use invalid configuration too, right?

That depends. Are they also using linting tests which force a particular platform-specific type of line ending? Or do they adapt those properly, like you are not?

[alexander-akait]

@rjgotten https://github.com/facebook/react/blob/master/scripts/prettier/index.js, nothing about line ending

[alexander-akait]

What about recommendations from github https://help.github.com/en/articles/configuring-git-to-handle-line-endings?

[rjgotten]

Ironic that you would recommend that, since:

text eol=lf Git will always convert line endings to LF on checkout. You should use this for files that must keep LF endings, even on Windows.

And considering lint:prettier is configured to enforce LF line endings on everything, it means all files must indeed use LF endings...

[alexander-akait]

@rjgotten

Ironic that you would recommend that, since:

I don't say this

lint:prettier

prettier works only for specific files (js, css, html, markdown, see prettier docs for full list), for example you can put test/issue-589/test.txt with LF/CRLF/CR because sometimes we have very rare edge cases (problems) with line ending and we should test them

[alexander-akait]

core.autocrlf=false

From github recommendations (https://help.github.com/en/articles/configuring-git-to-handle-line-endings)

Global settings for line endings The git config core.autocrlf command is used to change how Git handles line endings. It takes a single argument.

On Windows, you simply pass true to the configuration. For example:

$ git config --global core.autocrlf true

Configure Git on Windows to properly handle line endings

[rjgotten]

autocrlf=true would convert all LF endings in the repository to CRLF and cause it to fail the lint tests, which require LF.

How is that recommended, in this case, in any sense of the word?

[alexander-akait]

@rjgotten this is recommendation from github :confused:

[rjgotten]

The actual recommendation from Github is:

text eol=lf Git will always convert line endings to LF on checkout. You should use this for files that must keep LF endings, even on Windows.

Let me re-emphasize on that last bit:

You should use this for files that must keep LF endings, even on Windows.

And that is a hard requirement here, considering your prettier configuration.

[alexander-akait]

text=auto Git will handle the files in whatever way it thinks is best. This is a good default option.

[rjgotten]

Ok. I'm done arguing in circles over this. At this point it is less effort to adjust my global git configuration to appease your broken settings than it is to convince you to fix anything.

... something; something ... brick wall ... something ...

[alexander-akait]

@rjgotten broken settings :smile: you have invalid core.autocrlf=false option and no one is to blame for this except for you, and yes, I don’t want to continue this talk, you are so toxic,

We are working on many repos for you and other developers, help you and other developers, answer questions, fixing bugs and implement new feature, sometimes it is not easy, but listen that we are all wrong here and we do everything wrong is not motivated to help you.

Thanks and goodbye