Ignoring of node_modules doesn't work on certain packages about markdownlint-cli HOT 14 CLOSED

I can't try this from my phone, but version 0.34.0 was released about 10 months ago and this is pretty fundamental to be broken that long without anyone else noticing. Current version is 0.39.0 - could you please try that and confirm the version of this and Node.js (both via --version on the command line) that you are using?

from markdownlint-cli.

Markdownlint version: 0.39.0
Node version: v20.11.0

from markdownlint-cli.

@DavidAnson maybe related to the changes to support scoped packages e785e42 ?

from markdownlint-cli.

I do not observe the reported behavior. Here's what I did in a new GitHub Codespace for this repo after deleting all files. Note that I explicitly included the package shown in the example above. Please review and provide specific steps to reproduce the behavior you observe, ideally by pointing to a repository/commit that demonstrates the problem.

@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ node --version
v20.11.0
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm --version
10.2.4
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ ls -a
.  ..  .git  package.json
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ cat package.json
{
  "dependencies": {
    "markdownlint-cli": "0.39.0",
    "@optimizely/js-sdk-logging": "0.3.1"
  },
  "scripts": {
    "version": "markdownlint --version",
    "include": "markdownlint '**/*.md'",
    "exclude": "markdownlint '**/*.md' --ignore node_modules"
  }
}
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm install

...
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm run version

> version
> markdownlint --version

0.39.0
@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm run include

> include
> markdownlint '**/*.md'

node_modules/@isaacs/cliui/README.md:71:1 MD033/no-inline-html Inline HTML [Element: img]
...
node_modules/@optimizely/js-sdk-logging/README.md:15 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]
...
node_modules/wrap-ansi/readme.md:90:1 MD010/no-hard-tabs Hard tabs [Column: 1]

@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ npm run exclude

> exclude
> markdownlint '**/*.md' --ignore node_modules

@DavidAnson ➜ /workspaces/markdownlint-cli (master) $ 

from markdownlint-cli.

Hi,
I setup a new repo: https://github.com/ColinMorris83/markdownlint-testing
I have tried this with both Windows and on Macbook.

On both, the include script lists hundreds of errors covering multiple packages.
On both, the exclude script only lists errors from node_modules/@optimizely

from markdownlint-cli.

@ColinMorris83, FYI, I created a PR to ignore node_modules/ by default: #460.

from markdownlint-cli.

@ColinMorris83, thanks, I will try tonight via Codespaces. To be clear, you originally said that ignore of node_modules didn't work (at all?), but your latest comment seems to say it's only this one package within node_modules that's not being ignored in your scenario? (If so, that rules out certain possibilities.)

from markdownlint-cli.

@ColinMorris83, thanks, I will try tonight via Codespaces. To be clear, you originally said that ignore of node_modules didn't work (at all?), but your latest comment seems to say it's only this one package within node_modules that's not being ignored in your scenario? (If so, that rules out certain possibilities.)

Hey, yeah it looks like it does mostly work, but doesn't work for this particular package. I initially thought perhaps it was because it started with an @ but I don't think that is the cause because there are other packages also starting with that which do show errors when not ignoring node_modules, but it's only the optimizely one still having errors when node_modules is being ignored.

from markdownlint-cli.

Codespaces demonstrates the expected/correct behavior for your repo. I created a new Codespace via the GitHub UI, let it automatically install dependencies, then ran the following in the default bash terminal:

@DavidAnson ➜ /workspaces/markdownlint-testing (main) $ npm run exclude

> [email protected] exclude
> markdownlint "**/*.md" --ignore node_modules

@DavidAnson ➜ /workspaces/markdownlint-testing (main) $ 

Whatever's happening to you seems to be unique to your configuration. Codespaces are nice because they provide a clean, reproducible environment. Please see if you can update your repository to reproduce the issue - and you may find what's special about your environment in the process.

from markdownlint-cli.

Thanks, yeah if I do a codespace off the repo, it works ok. However, I've got 2 other people to also try checking out the repo and running locally, and they are getting the same issue as me. I've also got it happening on both my work macbook and my home PC on windows.

What does work is changing the script to this:
"exclude": "markdownlint \"**/*.md\" --ignore \"node_modules/**\""

So I'll use that for now

from markdownlint-cli.

Great. FYI, because we agree this does not reproduce with the repository you created (via Codespaces) and because there is a unit test for directory ignore that passes on Mac, Ubuntu, and Windows (

from markdownlint-cli.

I think I spot what's occurring now:

The joined path when ignore is set to just node_modules is this:

node_modules/**/*.{md,markdown}

And if you look at the errors:
node_modules/@optimizely/js-sdk-logging/CHANGELOG.MD:1 MD022

The extension is capital MD

I tried renaming that to lowercase md, and that resolves it.

So looks to be a casing issue

from markdownlint-cli.

Great find! I will look into this later when I have time. I'm not sure how that file got the "wrong" casing on your machine, but I don't see any reason why the ignore pattern should be case-sensitive. I should be able to change that safely so this won't be a problem for you again.

from markdownlint-cli.

It looks like those packages that have it with capitals have not been published in a long time, but at their last publish, they contained the capitals. e.g. https://github.com/optimizely/javascript-sdk/tree/3.2.x/packages/utils

If it helps, I tried adding in nocase: true to the minimatch match options and that seems to do the trick, but there may be better/other ways of handling it

return previousResults.filter(fileInfo => matcher.match(fileInfo.absolute, { nocase: true })).map(fileInfo => fileInfo.original);

Just did a bit of further digging, looks like they did correct the file naming: optimizely/javascript-sdk@5d526bd but haven't actually published again to npm, so we're stuck with it being incorrectly named.

from markdownlint-cli.