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

Add include syntax #58

Open
remcohaszing opened this issue Apr 9, 2020 · 9 comments
Open

Add include syntax #58

remcohaszing opened this issue Apr 9, 2020 · 9 comments

Comments

@remcohaszing
Copy link

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
Copy link
Owner

kaelzhang commented Apr 9, 2020

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
Copy link
Owner

kaelzhang commented Apr 9, 2020

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
Copy link
Owner

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

@blaggacao
Copy link

blaggacao commented Jul 17, 2020

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 kaelzhang reopened this Jul 17, 2020
@kaelzhang
Copy link
Owner

kaelzhang commented Jul 17, 2020

@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
Copy link
Author

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
Copy link
Owner

@remcohaszing Great idea 👍

@ajotaos
Copy link

ajotaos commented Nov 10, 2020

This a great contribution for everyone, love this proposal 👍

@jedwards1211
Copy link

jedwards1211 commented Nov 28, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants