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

fix: ignore-from-file relative to config file #701

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jellehelsen
Copy link

This fixes the problem (#673) that files mentioned in ignore-from-file can not be found when running in a subdirectory. Values in ignore-from-file are now treated as being relative to the config file.

@coveralls
Copy link

coveralls commented Nov 26, 2024

Coverage Status

coverage: 99.827% (+0.002%) from 99.825%
when pulling 174a64d on jellehelsen:fix/relative-ignore-from-file
into e118296 on adrienverge:master.

@adrienverge
Copy link
Owner

Hello!

Depending on the outcome of #673 and what @ndrwnaguib thinks, this PR could be welcome and merged 👍

I haven't reviewed yet, but I can already ask:

  • Can you please minimize all the diff that can be? E.g. don't change config to c; keep validate_rule_conf() independant (not in a class) and pass it config_dir if needed; etc.
  • Make the commit clean and ready to review. For example I saw a self.maxDiff = None.
  • Document this new behavior by updating documentation, for example inside a new note below this one: https://github.com/adrienverge/yamllint/blob/8513d9b/docs/configuration.rst?plain=1#L229
  • Please keep single quotes 🙏
  • Add test cases for at least:
      - an ignore-from-file: file when file is in the root dir but current dir is a sub dir (should find it)
      - an ignore-from-file: file when file is in a sub dir but current dir is this sub dir (should error)
      - an ignore-from-file: file when configuration is not passed via a file (found)
      - an ignore-from-file: file when configuration is not passed via a file (error)
      - other cases?

@jellehelsen jellehelsen force-pushed the fix/relative-ignore-from-file branch 4 times, most recently from 257daf9 to bb68cd0 Compare December 18, 2024 15:31
@jellehelsen jellehelsen force-pushed the fix/relative-ignore-from-file branch from bb68cd0 to 174a64d Compare December 18, 2024 15:36
@jellehelsen
Copy link
Author

Hi,

I think I've addressed all your remarks.

  • validate_rule_conf() is back out of the class and get passed the config_dir with None as default value, restored corresponding test
  • Tests added and existing tests cleaned.
  • Quotes restored (I should not have run ruff format. Apologies)
  • Added a line of documentation at the suggested location to indicate that the ignore-from-file path is relative to the config file.

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

Successfully merging this pull request may close these issues.

3 participants