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

Allow multiple config files #123

Closed
wants to merge 1 commit into from

Conversation

fetzerch
Copy link

@fetzerch fetzerch commented Aug 2, 2019

Extend '--config-file' command line option to support multiple config files. The configuration parameters get merged recursively.

This is useful to ship a definition of 'additional_commands' with a library of CMake modules while still allowing the user to provide his own project specific config file.

@cheshirekow cheshirekow self-assigned this Aug 2, 2019
@cheshirekow cheshirekow added the feature-request Report requests some new capability or enhancement label Aug 2, 2019
@cheshirekow
Copy link
Owner

Hi! Thanks for the contribution. I think I agree that there's a need for multiple config loads so that you can get libraries of command specifications. A quick glance of the code looks good. I will follow up soon.

@fetzerch fetzerch force-pushed the multiple_configs branch 2 times, most recently from 8a4a0c3 to 155f3b7 Compare August 5, 2019 07:51
Extend '--config-file' command line option to support multiple config
files. The configuration parameters get merged recursively.

This is useful to ship a definition of 'additional_commands' with
a library of CMake modules while still allowing the user to provide his
own project specific config file.
@cheshirekow
Copy link
Owner

Just an FYI: I'm going to follow this up with some kind of include config file option. I think that rather than specifying multiple config files on the command line, it would be nicer for one config file to be able to refer to another.

@JohelEGP
Copy link

I'm working on #201 and will soon make a WIP PR for it at https://github.com/TheLartians/Format.cmake/. Note that the configuration parameters are not being merged recursively. So only the last config with additional_commands is taken into account and previous ones discarded.

@cheshirekow
Copy link
Owner

Hi @JohelEGP, I think the dictionary merge implementation should be merging the additional commands. If not, that sounds like a bug.

@JohelEGP
Copy link

Then it seems like there's a bug. Should I open a new issue for that?

@cheshirekow
Copy link
Owner

Yes please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Report requests some new capability or enhancement fix-in-flight
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants