node-ignore icon indicating copy to clipboard operation
node-ignore copied to clipboard

Add include syntax

Open remcohaszing opened this issue 4 years ago • 9 comments

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.

remcohaszing avatar Apr 09 '20 13:04 remcohaszing

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 avatar Apr 09 '20 14:04 kaelzhang

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 avatar Apr 09 '20 14:04 kaelzhang

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

kaelzhang avatar May 19 '20 11:05 kaelzhang

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.

blaggacao avatar Jul 17 '20 04:07 blaggacao

@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.

kaelzhang avatar Jul 17 '20 13:07 kaelzhang

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.

remcohaszing avatar Jul 17 '20 13:07 remcohaszing

@remcohaszing Great idea 👍

kaelzhang avatar Jul 17 '20 14:07 kaelzhang

This a great contribution for everyone, love this proposal 👍

ajotaos avatar Nov 10 '20 14:11 ajotaos

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.

jedwards1211 avatar Nov 28 '20 03:11 jedwards1211