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

Kdl include syntax #3899

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

pkleineb
Copy link

@pkleineb pkleineb commented Dec 29, 2024

These changes aim to provide an include keyword in the kdl config files that provides support for spreading configuration over multiple files as discussed in #3376.

What I did:

  • added include keyword to the kdl config
  • the include keyword parses the included file(s) just like the main.kdl file gets parsed
  • include keyword can take either a single file path as a string as an argument like this:
    include "path/to/kdl/config/file.kdl
    
    or take multiple paths as a list of strings:
    include {
      "path1/to/file.kdl"
      "path2/to/file.kdl"
    }
    
  • the include keyword may take absolute or relative paths and does expand shell variables using shellexpand

What I didn't implement(maybe yet):

  • include can only be written once per file similar to all the other config options
  • most other config structs implement a to_kdl method which isn't implemented for include although I feel like it would be quite difficult
  • I only implemented some quick tests that might not test all cases (let me know if I should improve on that)

I hope this summarizes everything for you guys. Tell me if I can improve on something since I am still learning rust and not all too well versed in the language. I was also not too sure where to put my functions so tell me if they should be moved. Also tell me if I should squash my 3 commits no problem. Also feel free to tell me if this shouldn't be merged to main, wasn't quite sure now.

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.

1 participant