fluid-project / fluid-lint-all

Consolidated linting logic free from any particular build technology
BSD 3-Clause "New" or "Revised" License
0 stars 5 forks source link

Exclude files in .gitignore from linting #3

Closed greatislander closed 3 years ago

greatislander commented 3 years ago

Is your feature request related to a problem?

As per https://github.com/fluid-project/fluid-lint-all/pull/2#discussion_r558438411, fluid-lint-all currently lints generated files which are listed in a project's .gitignore file.

Describe the solution you'd like

I think it's safe to assume that fluid-lint-all should ignore any files present in .gitignore by default. I think it's also good to ensure that:

Describe alternative solutions you've considered

Not applicable.

Additional context or notes

Not applicable.

the-t-in-rtf commented 3 years ago

We chatted about the basic idea of this, which I had an approach for. However, this is a bit of a challenge that I need to think through:

Explicitly naming a file in a linter's includes configuration block would cause it to be linted even if it was present in .gitignore

Carving out this kind of exception would require converting from a single pass (includes and excludes), to a multi-stage approach. For example:

  1. Read in the gitignore file.
  2. Convert from its pattern syntax to ours.
  3. Exclude any patterns that match our includes.
  4. Add these filtered exclude patterns to the excludes defined in our options.
  5. Search recursively for files that match our includes and the calculated excludes.

I'm not 100% sure I can manage point two all that well, but will read up on the pattern syntax used in .gitignore files. Another hitch is that the matching we used is based around comparing a path string to include patterns and exclude patterns, rather than comparing another pattern. I'm not 100% positive that I can correctly unpack partially overlapping patterns, so probably another approach is needed.

Another approach would be too come up with the list of files as before and then exclude the ones that match the .gitignore patterns, but this would not satisfy your requirement, i.e. there would be no way to opt a file back into the checking, i.e. I still have no way of filtering the .gitignore patterns based on the includes.

Things will be clearer and simpler if we can agree to live without this one bit. For people who genuinely want to lint select material in their .gitignore, they can turn off the feature, and perhaps use a separate configuration file for that purpose.

the-t-in-rtf commented 3 years ago

I probably won't get to it today, but have at least the core of the idea. I think I'll keep the .gitignore pattern matching out of fluid-glob and use minimatch more directly. So I'd reuse our minimatch options and individually check each path against each "exclude" coming from .gitignore.

That just leaves me needing to parse the file itself, which seems to just be a matter of ignoring hashed comment lines and ignoring blank lines. Everything else minimatch should take care of for us.