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

📝 Ignore option not respected #3345

Open
1 task done
marvinhagemeister opened this issue Jul 3, 2024 · 8 comments
Open
1 task done

📝 Ignore option not respected #3345

marvinhagemeister opened this issue Jul 3, 2024 · 8 comments
Assignees
Labels
A-CLI Area: CLI S-Bug-confirmed Status: report has been confirmed as a valid bug S-Help-wanted Status: you're familiar with the code base and want to help the project workaround
Milestone

Comments

@marvinhagemeister
Copy link

marvinhagemeister commented Jul 3, 2024

Environment information

CLI:
  Version:                      1.8.3
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v20.13.1"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "yarn/1.22.22"

Biome Configuration:
  Error:                        The property trailingComma is deprecated. Use javascript.formatter.trailingCommas instead.
  Status:                       Loaded with errors
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Formatter:
  Format with errors:           false
  Indent style:                 Tab
  Indent width:                 2
  Line ending:                  Lf
  Line width:                   80
  Attribute position:           Auto
  Ignore:                       ["**/.DS_Store", "**/node_modules", "**/npm-debug.log", "**/dist/", "*/package-lock.json", "**/yarn.lock", "**/.vscode", "**/.idea", "test/ts/**/*.js", "**/coverage", "**/*.sw[op]", "**/*.log", "**/package/", "**/preact-*.tgz", "**/preact.tgz", "benches/dist/", "benches/results/", "benches/logs/", "benches/logs-saved/", "benches/node_modules/", "benches/proxy-packages/*/package-lock.json", "**/package-lock.json"]
  Include:                      []

JavaScript Formatter:
  Enabled:                      false
  JSX quote style:              Double
  Quote properties:             AsNeeded
  Trailing commas:              None
  Semicolons:                   Always
  Arrow parentheses:            AsNeeded
  Bracket spacing:              true
  Bracket same line:            false
  Quote style:                  Single
  Indent style:                 unset
  Indent width:                 unset
  Line ending:                  unset
  Line width:                   unset
  Attribute position:           Auto

JSON Formatter:
  Enabled:                      true
  Indent style:                 unset
  Indent width:                 unset
  Line ending:                  unset
  Line width:                   unset
  Trailing Commas:              unset

CSS Formatter:
  Enabled:                      false
  Indent style:                 unset
  Indent width:                 unset
  Line ending:                  unset
  Line width:                   unset
  Quote style:                  Double

Workspace:
  Open Documents:               0

Configuration

{
  "formatter": {
    "enabled": true,
    "formatWithErrors": false,
    "indentStyle": "tab",
    "indentWidth": 2,
    "lineEnding": "lf",
    "lineWidth": 80,
    "attributePosition": "auto",
    "ignore": [
      "**/.DS_Store",
      "**/node_modules",
      "**/npm-debug.log",
      "**/dist",
      "*/package-lock.json",
      "**/yarn.lock",
      "**/.vscode",
      "**/.idea",
      "test/ts/**/*.js",
      "**/coverage",
      "**/*.sw[op]",
      "**/*.log",
      "**/package/",
      "**/preact-*.tgz",
      "**/preact.tgz",
      "benches/dist/",
      "benches/results/",
      "benches/logs/",
      "benches/logs-saved/",
      "benches/node_modules/",
      "benches/proxy-packages/*/package-lock.json",
      "**/package-lock.json"
    ]
  },
  "organizeImports": { "enabled": true },
  "linter": { "enabled": true, "rules": { "recommended": true } },
  "javascript": {
    "formatter": {
      "jsxQuoteStyle": "double",
      "quoteProperties": "asNeeded",
      "trailingComma": "none",
      "semicolons": "always",
      "arrowParentheses": "asNeeded",
      "bracketSpacing": true,
      "bracketSameLine": false,
      "quoteStyle": "single",
      "attributePosition": "auto"
    }
  },
  "overrides": [
    {
      "include": ["*.json", ".*rc", "*.yml"],
      "formatter": { "indentWidth": 2, "indentStyle": "space" }
    }
  ]
}

Playground link

Steps to reproduce:

  1. Clone https://github.com/preactjs/preact
  2. Run npm i
  3. Run npm run build
  4. Run biome format .

It will try to format all generated files in dist/ folders despite them being ignored in the config.

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@ematipico ematipico added A-CLI Area: CLI S-Bug-confirmed Status: report has been confirmed as a valid bug S-Help-wanted Status: you're familiar with the code base and want to help the project labels Jul 4, 2024
@Conaclos
Copy link
Member

Conaclos commented Jul 5, 2024

Have you tried **/dist/** instead of **/dist?

@ematipico
Copy link
Member

That will work, but I think it's a workaround. **/dist should be still valid I think. Looking at how they run the command, they use biome format ., so the folder will be crawled with a path like this ./dist, which I think should match the pattern, however we know there's a bug filed (it's somewhere), and we came to the conclusion that we prefer to evaluate a better glob lib.

@Conaclos
Copy link
Member

Conaclos commented Jul 5, 2024

We have #2000.

Maybe it is a regression, because I remember making a change to ignore entire folders when specified under ignore. We should try the reproduction on different versions of Biome.

@marvinhagemeister
Copy link
Author

When I add the folders explicitly they are ignored as expected:

    "ignore": [
-     "**/dist/",
+     "hooks/dist/",
+     "debug/dist/",
+     "compat/dist/",
+     "dist/",
    ]

Seems like the leading ** are not working, or maybe I'm misunderstanding the syntax.

@Sec-ant
Copy link
Member

Sec-ant commented Jul 6, 2024

While I was working on #3317, I noticed some code that might be related to this issue:

// Here we cover cases where the user specifies single files inside the patterns.
// The pattern library doesn't support single files, we here we just do a check
// on contains
//
// Given the pattern `out`:
// - `out/index.html` -> matches
// - `out/` -> matches
// - `layout.tsx` -> does not match
// - `routes/foo.ts` -> does not match
source
.ancestors()
.any(|ancestor| ancestor.ends_with(pattern.as_str()))

I think it was meant to be starts_with, but that will still match many false positives.

And I can't see where the root field in the Matcher is used.

@Conaclos
Copy link
Member

Conaclos commented Jul 7, 2024

We have many issues with the glob matcher library. This is something we want to improve in Biome 2.0 I started some experimentation two month ago.

@mattfysh
Copy link

mattfysh commented Jul 8, 2024

one thing that could be fixed is how "foo/**/*" will match both <rootDir>/foo/x and <rootDir>/nested/dir/foo/x. there's currently no way to clamp down the glob to say just the /foo at root

the only workaround i've found so far is using the parent directory name, e.g. my_repo_dir/foo/**/*

@Conaclos Conaclos added this to the Biome 2.0 milestone Sep 3, 2024
@emilte
Copy link

emilte commented Sep 18, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI S-Bug-confirmed Status: report has been confirmed as a valid bug S-Help-wanted Status: you're familiar with the code base and want to help the project workaround
Projects
None yet
Development

No branches or pull requests

6 participants