kaelzhang / node-ignore

🔍 node-ignore is the manager and filter for .gitignore rules, the one used by eslint, prettier and many others.
Other
440 stars 45 forks source link

Add include syntax #58

Open remcohaszing opened 4 years ago

remcohaszing commented 4 years ago

This package is used by various tools for parsing an ignore file. Since basically every tool, especially linter, simply needs to ignore files from .gitignore, and maybe some other files, it would be nice if they could simply include the file list from .gitignore.

In the Google Cloud SDK, this is solved using the following syntax:

#!include:.gitignore

See https://cloud.google.com/sdk/gcloud/reference/topic/gcloudignore

Currently Prettier, ESLint, and Stylelint all use separate ignore, all parsed using this package. All of their ignore files typically contain the same content.

kaelzhang commented 4 years ago

It is easy for us to implement this by extending node-ignore

.gcloudignore

.gcloudignore
 # If you would like to upload your .git directory, .gitignore file or
 # files from your .gitignore file, remove the corresponding line below:
 .git
 .gitignore
 #!include:.gitignore
const MATCH_INCLUDE = /#!include\s*:\s*([^\s]+)/

const gcloudignore = fs.readFileSync('.gcloudignore').toString()

const ig = ignore().add(gcloudignore)

gcloudignore
.split(/\r|\n/)
.map(line => {
  const match = line.match(MATCH_INCLUDE)
  return match && match[1]
})
.filter(Boolean)
.forEach(include => {
  ig.add(fs.readFileSync(include).toString())
})

ig.ignores(somePath)

It just takes 2 minutes, not tested.

kaelzhang commented 4 years ago

IMO, node-ignore is a pure parser to parse some ignore patterns which follow the .gitignore rules, and we'd better add any other features to it.

Even if I add a new feature of include syntax, I need to set the flag to false by default, or it might introduce some unexpected behavior, or even leads to a breaking change.

Maybe we could involve some guys from Prettier or Eslint for further discussion, or open a related issue to the Eslint repo.

kaelzhang commented 4 years ago

I am closing this. Be free to reopen this issue if necessary

blaggacao commented 4 years ago

I would consider such implementation here very benevolent upstream nudging.

A feature flag is maybe the right balance.

There is a strong need downstream as evidenced by the linked (recent) issues from prettier.

@kaelzhang I hope you might reconsider to implement this in order to cut on pointless (and eternal) downstream discussions.

If google does it and node-ignore does it, then maybe other language ecosystems will converge, too. The problem is generic enough.

kaelzhang commented 4 years ago

@blaggacao maybe I could provide another package which depends on node-ignore, that I want to keep node-ignore nothing to do with fs(file systems) because the package node-ignore is also widely used in browsers.

Any suggestions?

Months ago, I once had a plan to make a ignore parser which supports to handle .gitignore files in multiple directories, and has to depend on file systems.

remcohaszing commented 4 years ago

How about an include options which takes a referenced filename, and returns more ignore patterns. I.e.

const ig = ignore({
  include(filename) {
    return fs.readFileSync(filename, 'utf-8');
  }
})

If a line is encounted that starts with #!include:, the filename is passed int the include option. If the callback returns a string, this value is added to the ignore instance.

kaelzhang commented 4 years ago

@remcohaszing Great idea 👍

ajotaos commented 4 years ago

This a great contribution for everyone, love this proposal 👍

jedwards1211 commented 3 years ago

I'll probably make a higher-level package with a .loadIgnoreFile method soon, will let everyone know if I do. I also need to make a watcher that respects ignore files for several of my own tools.

EDIT: oh i wasn't actually reading that people have include directives in their files though. Note: you can tell prettier and eslint to use your gitignore with a CLI option.