Skip to content

improve get_extended_config_file with expansions #767

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

freehck
Copy link

@freehck freehck commented Jul 29, 2025

This PR improves get_extended_config_file function providing it with
the ability to expand variables and tilde so you can use more complex
values in extends:.

Example 1:

extends: ~/repos/github.com/freehck/linting-rules/.yamllint

Example 2:

extends: $HOME/repos/github.com/freehck/linting-rules/.yamllint

Example 3:

extends: ${HOME}/repos/github.com/freehck/linting-rules/.yamllint

This is useful to store company's linting rules in a separate
repository.

@coveralls
Copy link

coveralls commented Jul 29, 2025

Coverage Status

coverage: 99.815%. remained the same
when pulling 31d9650 on freehck:config-filename-expansion
into 0cb7b0d on adrienverge:master.

@freehck
Copy link
Author

freehck commented Jul 29, 2025

Now I also improved code coverage a bit. You're welcome.

@adrienverge
Copy link
Owner

adrienverge commented Jul 31, 2025

Hello,

Thanks for proposing this contribution!

I'm not favorable to it, because:

  • Expansion of variable names starting with $ is Shell-specific, it's not a POSIX or UNIX standard.
  • $ is a valid character in file paths (e.g. $$$ and $test are valid file names). This change would break behavior for users who have files like gimme$and€.
  • The added code is redundant with find_project_config_filepath().

I understand that your goal is to store custom linting rules in a separate repository. Have you tried letting your shell do the substitution, e.g. something like this?

yamllint -d "extends: $HOME/repos/github.com/freehck/linting-rules/.yamllint" .

Now I also improved code coverage a bit. You're welcome.

Thank you 😄

@freehck
Copy link
Author

freehck commented Aug 1, 2025

Okay, I understand your point: as variables are considered to be redundant due to the fact users can have $ in file names, then I agree. Let's get rid of it.

But what's about ~?

I understand that your goal is to store custom linting rules in a separate repository. Have you tried letting your shell do the substitution, e.g. something like this?
yamllint -d "extends: $HOME/repos/github.com/freehck/linting-rules/.yamllint" .

This won't help me a lot. What I tried to achieve is the ability to define the default profile for my company that can be used in different repos. This implies that different repos can use the same basis (default profile) but also provide repository-specific additions.

F.e. in my ansible repo I have the following:

---
extends: ~/repos/github.com/almaops/linting-rules/yamllint/main.yml

ignore:
  - collections/
  - roles/
  - "**/files"
  - "**/templates"
...

So rules, as you can see, are common and taken from ~/repos/github.com/almaops/linting-rules/yamllint/main.yml. But the ignore directive is specific to the repo. Some other repository can have one or several of these directories (like files/) but it's not intended to ignore them there.

Another example. I've put this into my terraform repo:

---
extends: ~/repos/github.com/almaops/linting-rules/yamllint/main.yml

rules:
  document-start: disable

This specific additional rule is necessary because this repo has a bunch of yaml files that are already deployed to the production environment, and it'd redundant to enforce helm charts redeployment just because yaml files don't have --- in the beginning.

So the situation is the same: common rules are taken from ~/repos/github.com/almaops/linting-rules/yamllint/main.yml, but it has sense to have the additional rule to disable document-start for this specific repo but not for others.


Therefore my proposal is the following:

What would you think if I removed variables expansion but leaved tilde expansion?

@freehck
Copy link
Author

freehck commented Aug 1, 2025

To clarify: I understand that ~ can be used in file names too. But probably let's have it just for paths starting with it? Just as a hack for now.

Probably it'd be a better solution if we provide yamllint with the ability to specify the directory with our own profiles. But I didn't think a lot about this.

@adrienverge
Copy link
Owner

What would you think if I removed variables expansion but leaved tilde expansion?

Like $, ~ is shell-specific and can be used in filenames (even just a one-char ~ is a valid filename): better let the shell decide whether to expand or not, and not mess with that in yamllint.

I think I understood your 2 real-life examples, although to be honest I'm not 100% sure. Anyway, if your goal is to extend different yamllint configurations depending on which user is running it, why do you need the ~, why don't you just use the current working directory?

$ su userA
$ pwd
/home/userA
$ yamllint -d "{extends: repos/…/main.yaml, ignore: […]}"
# yamllint should extend config from /home/userA/repos/…/main.yaml
# and compute ignore files from /home/userA
$ su userB
$ pwd
/home/userB
$ yamllint -d "{extends: repos/…/main.yaml, ignore: […]}"
# yamllint should extend config from /home/userB/repos/…/main.yaml
# and compute ignore files from /home/userB

@freehck
Copy link
Author

freehck commented Aug 4, 2025

yamllint -d "{extends: repos/…/main.yaml, ignore: […]}"

Because it's inconvenient. In most cases we expect that a simple run of yamllint . in a repo directory will do the thing. All the pipelines and user practices rely on it. Additional flags are possible, but we don't want them to be different in different repos: it will become quite complicated.

better let the shell decide whether to expand or not, and not mess with that in yamllint

Well okay, I got your point. So the suggested changes are considered as shell expansions and as my PR modifies this default behaviour it can harm users who have $ and ~ in file names.

But I need it. Probably there're users who need it too. So I come with a new proposal. What if I add a new yamllint option, smth like --enable-shell-expansion activating shell expansion for extends directive?

Then we'll keep the default behaviour and allow users to activate shell expansion if they want. How do you feel about it?

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